[gjs: 1/3] cairo: Take may_be_null into account in foreign struct conversion

[Cosimo Cecchi <cosimoc src gnome org>]

commit 17021714193d79b1c8031ea4f02a8243b0ec14fc
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Apr 20 15:04:27 2019 -0700

    cairo: Take may_be_null into account in foreign struct conversion
    
    Previously, when converting from a Cairo struct to a GIArgument, the
    "may_be_null" parameter would be ignored and a null pointer was always
    accepted (and then would probably crash, although this seems to be an
    unlikely case as we never got any bug reports of it.)
    
    Instead, we do a null check, and throw a JS exception if needed, in the
    foreign struct conversion callbacks in the various Cairo types.
    
    Requires exposing a previously static function in arg.cpp so that the
    thrown exceptions can have more useful messages.
    
    This bug was found with -Wunused-parameter.

 gi/arg.cpp                | 29 +++++++++++------------------
 gi/arg.h                  |  6 +++++-
 modules/cairo-context.cpp | 14 +++++++++++++-
 modules/cairo-region.cpp  | 12 ++++++++++++
 modules/cairo-surface.cpp | 12 ++++++++++++
 5 files changed, 53 insertions(+), 20 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 67265c1e..7e060924 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1221,11 +1221,8 @@ gjs_g_array_new_for_type(JSContext    *context,
     return g_array_sized_new(true, false, element_size, length);
 }
 
-GJS_USE
-static gchar *
-get_argument_display_name(const char     *arg_name,
-                          GjsArgumentType arg_type)
-{
+char* gjs_argument_display_name(const char* arg_name,
+                                GjsArgumentType arg_type) {
     switch (arg_type) {
     case GJS_ARGUMENT_ARGUMENT:
         return g_strdup_printf("Argument '%s'", arg_name);
@@ -1273,12 +1270,11 @@ throw_invalid_argument(JSContext      *context,
                        const char     *arg_name,
                        GjsArgumentType arg_type)
 {
-    gchar *display_name = get_argument_display_name(arg_name, arg_type);
+    GjsAutoChar display_name = gjs_argument_display_name(arg_name, arg_type);
 
     gjs_throw(context, "Expected type %s for %s but got type '%s'",
-              type_tag_to_human_string(arginfo),
-              display_name, JS::InformalValueTypeName(value));
-    g_free(display_name);
+              type_tag_to_human_string(arginfo), display_name.get(),
+              JS::InformalValueTypeName(value));
 }
 
 GJS_JSAPI_RETURN_CONVENTION
@@ -2018,21 +2014,18 @@ _Pragma("GCC diagnostic pop")
         }
         return false;
     } else if (G_UNLIKELY(out_of_range)) {
-        gchar *display_name = get_argument_display_name (arg_name, arg_type);
+        GjsAutoChar display_name =
+            gjs_argument_display_name(arg_name, arg_type);
         gjs_throw(context, "value is out of range for %s (type %s)",
-                  display_name,
-                  g_type_tag_to_string(type_tag));
-        g_free (display_name);
+                  display_name.get(), g_type_tag_to_string(type_tag));
         return false;
     } else if (nullable_type &&
                arg->v_pointer == NULL &&
                !may_be_null) {
-        gchar *display_name = get_argument_display_name (arg_name, arg_type);
-        gjs_throw(context,
-                  "%s (type %s) may not be null",
-                  display_name,
+        GjsAutoChar display_name =
+            gjs_argument_display_name(arg_name, arg_type);
+        gjs_throw(context, "%s (type %s) may not be null", display_name.get(),
                   g_type_tag_to_string(type_tag));
-        g_free (display_name);
         return false;
     } else {
         return true;
diff --git a/gi/arg.h b/gi/arg.h
index b60d4108..8cbdae2f 100644
--- a/gi/arg.h
+++ b/gi/arg.h
@@ -34,7 +34,8 @@
 
 G_BEGIN_DECLS
 
-/* Different roles for a GArgument */
+// Different roles for a GIArgument; currently used only in exception and debug
+// messages.
 typedef enum {
     GJS_ARGUMENT_ARGUMENT,
     GJS_ARGUMENT_RETURN_VALUE,
@@ -44,6 +45,9 @@ typedef enum {
     GJS_ARGUMENT_ARRAY_ELEMENT
 } GjsArgumentType;
 
+GJS_USE
+char* gjs_argument_display_name(const char* arg_name, GjsArgumentType arg_type);
+
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_value_to_arg(JSContext      *context,
                       JS::HandleValue value,
diff --git a/modules/cairo-context.cpp b/modules/cairo-context.cpp
index 738b9401..21467ff6 100644
--- a/modules/cairo-context.cpp
+++ b/modules/cairo-context.cpp
@@ -976,7 +976,19 @@ context_to_g_argument(JSContext      *context,
                       bool            may_be_null,
                       GArgument      *arg)
 {
-    JS::RootedObject obj(context, value.toObjectOrNull());
+    if (value.isNull()) {
+        if (!may_be_null) {
+            GjsAutoChar display_name =
+                gjs_argument_display_name(arg_name, argument_type);
+            gjs_throw(context, "%s may not be null", display_name.get());
+            return false;
+        }
+
+        arg->v_pointer = nullptr;
+        return true;
+    }
+
+    JS::RootedObject obj(context, &value.toObject());
     cairo_t *cr;
 
     cr = gjs_cairo_context_get_context(context, obj);
diff --git a/modules/cairo-region.cpp b/modules/cairo-region.cpp
index cd9d31fa..33fbe86d 100644
--- a/modules/cairo-region.cpp
+++ b/modules/cairo-region.cpp
@@ -314,6 +314,18 @@ region_to_g_argument(JSContext      *context,
                      bool            may_be_null,
                      GArgument      *arg)
 {
+    if (value.isNull()) {
+        if (!may_be_null) {
+            GjsAutoChar display_name =
+                gjs_argument_display_name(arg_name, argument_type);
+            gjs_throw(context, "%s may not be null", display_name.get());
+            return false;
+        }
+
+        arg->v_pointer = nullptr;
+        return true;
+    }
+
     JS::RootedObject obj(context, &value.toObject());
     cairo_region_t *region;
 
diff --git a/modules/cairo-surface.cpp b/modules/cairo-surface.cpp
index 6b671ba5..b726db50 100644
--- a/modules/cairo-surface.cpp
+++ b/modules/cairo-surface.cpp
@@ -255,6 +255,18 @@ surface_to_g_argument(JSContext      *context,
                       bool            may_be_null,
                       GArgument      *arg)
 {
+    if (value.isNull()) {
+        if (!may_be_null) {
+            GjsAutoChar display_name =
+                gjs_argument_display_name(arg_name, argument_type);
+            gjs_throw(context, "%s may not be null", display_name.get());
+            return false;
+        }
+
+        arg->v_pointer = nullptr;
+        return true;
+    }
+
     JSObject *obj;
     cairo_surface_t *s;