[gjs: 1/3] function: Handle case of a bare GDestroyNotify in a function's args

[Cosimo Cecchi <cosimoc src gnome org>]

commit 7d241743aa42b407c5538ae8d4af9d486dc27fe9
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Mar 9 18:37:02 2019 -0800

    function: Handle case of a bare GDestroyNotify in a function's args
    
    For example, g_memory_input_stream_new_from_data() has a GDestroyNotify
    parameter which isn't associated with a callback, but with static data.
    I don't know of an annotation which will express the relation of a
    destroy parameter to anything other than a callback.
    
    Previously, since we assumed that all GDestroyNotify parameters were
    associated with callbacks, we'd mark their type as PARAM_SKIPPED. Their
    GIArgument would later be filled in when processing the calllback
    parameter. In the case of g_memory_input_stream_new_from_data(), this
    would crash because there was no callback parameter, so the GIArgument
    of the destroy parameter would never be filled in, and uninitialized
    data would be passed to the C function.
    
    Instead, mark such parameters as PARAM_UNKNOWN, a new parameter type
    indicating that we don't know what to pass for that parameter. If JS
    code tries to call a function with a PARAM_UNKNOWN parameter, then an
    exception will be thrown.
    
    Closes: #221

 gi/function.cpp                         | 23 +++++++++++++++++++++--
 gi/function.h                           |  3 ++-
 installed-tests/js/testGIMarshalling.js |  9 +++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index ce5f97b9..9380b62a 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -313,6 +313,8 @@ gjs_callback_closure(ffi_cif *cif,
             case PARAM_CALLBACK:
                 /* Callbacks that accept another callback as a parameter are not
                  * supported, see gjs_callback_trampoline_new() */
+            case PARAM_UNKNOWN:
+                // PARAM_UNKNOWN is currently not ever set on a callback's args.
             default:
                 g_assert_not_reached();
         }
@@ -1040,6 +1042,18 @@ gjs_invoke_c_function(JSContext                             *context,
                 break;
             }
 
+            case PARAM_UNKNOWN:
+                gjs_throw(context,
+                          "Error invoking %s.%s: impossible to determine what "
+                          "to pass to the '%s' argument. It may be that the "
+                          "function is unsupported, or there may be a bug in "
+                          "its annotations.",
+                          g_base_info_get_namespace(function->info),
+                          g_base_info_get_name(function->info),
+                          g_base_info_get_name(&arg_info));
+                failed = true;
+                break;
+
             default:
                 ;
             }
@@ -1661,8 +1675,13 @@ init_cached_function_data (JSContext      *context,
             if (interface_type == GI_INFO_TYPE_CALLBACK) {
                 if (strcmp(g_base_info_get_name(interface_info), "DestroyNotify") == 0 &&
                     strcmp(g_base_info_get_namespace(interface_info), "GLib") == 0) {
-                    /* Skip GDestroyNotify if they appear before the respective callback */
-                    function->param_types[i] = PARAM_SKIPPED;
+                    // We don't know (yet) what to do with GDestroyNotify
+                    // appearing before a callback. If the callback comes later
+                    // in the argument list, then PARAM_UNKNOWN will be
+                    // overwritten with PARAM_SKIPPED. If no callback follows,
+                    // then this is probably an unsupported function, so the
+                    // value will remain PARAM_UNKNOWN.
+                    function->param_types[i] = PARAM_UNKNOWN;
                 } else {
                     function->param_types[i] = PARAM_CALLBACK;
                     function->expected_js_argc += 1;
diff --git a/gi/function.h b/gi/function.h
index 42861d39..93cd7379 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -39,7 +39,8 @@ typedef enum {
     PARAM_NORMAL,
     PARAM_SKIPPED,
     PARAM_ARRAY,
-    PARAM_CALLBACK
+    PARAM_CALLBACK,
+    PARAM_UNKNOWN,
 } GjsParamType;
 
 struct GjsCallbackTrampoline {
diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js
index b6ed566c..27f6bca2 100644
--- a/installed-tests/js/testGIMarshalling.js
+++ b/installed-tests/js/testGIMarshalling.js
@@ -762,3 +762,12 @@ describe('GObject properties', function () {
         expect(() => obj.some_readonly = 35).toThrow();
     }).pend('https://gitlab.gnome.org/GNOME/gobject-introspection/merge_requests/32');
 });
+
+describe('GDestroyNotify parameters', function () {
+    it('throws when encountering a GDestroyNotify not associated with a callback', function () {
+        // the 'destroy' argument applies to the data, which is not supported in
+        // gobject-introspection
+        expect(() => Gio.MemoryInputStream.new_from_data('foobar'))
+            .toThrowError(/destroy/);
+    });
+});