[gjs/wip/ptomato/warnings: 9/16] js: Use std::vector instead of GArray

[Philip Chimento <pchimento src gnome org>]

commit cf200def2b44882175441bebd679dad7790f3fa7
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Oct 2 12:47:55 2016 -0700

    js: Use std::vector instead of GArray
    
    C++ does not approve of type-punning GArray's internal char *data
    pointer. This causes -Wcast-align because the alignment requirements on
    the underlying array may be different depending on which type it is.
    Casting char * to int * when the char array does not have int alignment
    may be slower on some architectures, or it may crash on some, for example
    older ARM architectures.
    
    We use std::vector instead since that does not have the same problem.
    In cairo-context.cpp we go ahead and switch to RAII style in the function
    where we replace GArray, since otherwise the gotos would cross the
    initialization boundary.
    
    In jsapi-util-array.cpp there is more GArray code but it is not actually
    used anywhere in the GJS codebase. There, we simply insert a pragma
    ignoring the warnings.

 gi/object.cpp             |   42 +++++++++---------------------------------
 gjs/jsapi-util-array.cpp  |    7 +++++++
 modules/cairo-context.cpp |   28 ++++++++++++----------------
 3 files changed, 28 insertions(+), 49 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index f54007f..27f35f2 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -25,6 +25,7 @@
 
 #include <memory>
 #include <string.h>
+#include <vector>
 
 #include <gjs/gi.h>
 #include "object.h"
@@ -659,7 +660,6 @@ free_g_params(GParameter *params,
     for (i = 0; i < n_params; ++i) {
         g_value_unset(&params[i].value);
     }
-    g_free(params);
 }
 
 /* Set properties from args to constructor (argv[0] is supposed to be
@@ -671,24 +671,14 @@ object_instance_props_to_g_parameters(JSContext   *context,
                                       unsigned     argc,
                                       JS::Value   *argv,
                                       GType        gtype,
-                                      GParameter **gparams_p,
-                                      int         *n_gparams_p)
+                                      std::vector<GParameter>& gparams)
 {
     JSObject *props;
     JSObject *iter;
     jsid prop_id;
-    GArray *gparams;
-
-    if (gparams_p)
-        *gparams_p = NULL;
-    if (n_gparams_p)
-        *n_gparams_p = 0;
-
-    gparams = g_array_new(/* nul term */ false, /* clear */ true,
-                          sizeof(GParameter));
 
     if (argc == 0 || argv[0].isUndefined())
-        goto out;
+        return true;
 
     if (!argv[0].isObject()) {
         gjs_throw(context, "argument should be a hash with props to set");
@@ -736,29 +726,17 @@ object_instance_props_to_g_parameters(JSContext   *context,
 
         g_free(name);
 
-        g_array_append_val(gparams, gparam);
+        gparams.push_back(gparam);
 
         prop_id = JSID_VOID;
         if (!JS_NextProperty(context, iter, &prop_id))
             goto free_array_and_fail;
     }
 
- out:
-    if (n_gparams_p)
-        *n_gparams_p = gparams->len;
-    if (gparams_p)
-        *gparams_p = (GParameter*) g_array_free(gparams, false);
-
     return true;
 
  free_array_and_fail:
-    {
-        GParameter *to_free;
-        int count;
-        count = gparams->len;
-        to_free = (GParameter*) g_array_free(gparams, false);
-        free_g_params(to_free, count);
-    }
+    free_g_params(&gparams[0], gparams.size());
     return false;
 }
 
@@ -1249,8 +1227,7 @@ object_instance_init (JSContext              *context,
 {
     ObjectInstance *priv;
     GType gtype;
-    GParameter *params;
-    int n_params;
+    std::vector<GParameter> params;
     GTypeQuery query;
     GObject *gobj;
 
@@ -1260,8 +1237,7 @@ object_instance_init (JSContext              *context,
     g_assert(gtype != G_TYPE_NONE);
 
     if (!object_instance_props_to_g_parameters(context, object, argc, argv,
-                                               gtype,
-                                               &params, &n_params)) {
+                                               gtype, params)) {
         return false;
     }
 
@@ -1276,9 +1252,9 @@ object_instance_init (JSContext              *context,
                               "object_init_list");
     }
 
-    gobj = (GObject*) g_object_newv(gtype, n_params, params);
+    gobj = (GObject*) g_object_newv(gtype, params.size(), &params[0]);
 
-    free_g_params(params, n_params);
+    free_g_params(&params[0], params.size());
 
     JS::RootedObject old_jsobj(context, peek_js_obj(gobj));
     if (old_jsobj != NULL && old_jsobj != object) {
diff --git a/gjs/jsapi-util-array.cpp b/gjs/jsapi-util-array.cpp
index 4bae378..8249f5d 100644
--- a/gjs/jsapi-util-array.cpp
+++ b/gjs/jsapi-util-array.cpp
@@ -26,6 +26,13 @@
 #include "jsapi-util.h"
 #include "compat.h"
 
+/* It seems impossible to use GArray in C++ without breaking alignment rules,
+ * since GArray's internal pointer is a char *. We will simply not use these
+ * anymore, instead use the JSAPI-provided JS::AutoValueVector. */
+#if defined(__clang__) || __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+_Pragma("GCC diagnostic ignored \"-Wcast-align\"")
+#endif
+
 /* Maximum number of elements allowed in a GArray of rooted JS::Values.
  * We pre-alloc that amount and then never allow the array to grow,
  * or we'd have invalid memory rooted if the internals of GArray decide
diff --git a/modules/cairo-context.cpp b/modules/cairo-context.cpp
index d7f2514..3d78b9e 100644
--- a/modules/cairo-context.cpp
+++ b/modules/cairo-context.cpp
@@ -22,6 +22,8 @@
 
 #include <config.h>
 
+#include <vector>
+
 #include <gjs/gjs-module.h>
 #include <gjs/compat.h>
 #include <gi/foreign.h>
@@ -568,55 +570,49 @@ setDash_func(JSContext *context,
     cairo_t *cr;
     JS::RootedObject dashes(context);
     double offset;
-    bool retval = false;
     guint len;
-    GArray *dashes_c = NULL;
 
     if (!gjs_parse_call_args(context, "setDash", "of", argv,
                         "dashes", dashes.address(), "offset", &offset))
         return false;
 
-
     if (!JS_IsArrayObject(context, dashes)) {
         gjs_throw(context, "dashes must be an array");
-        goto out;
+        return false;
     }
 
     if (!JS_GetArrayLength(context, dashes, &len)) {
         gjs_throw(context, "Can't get length of dashes");
-        goto out;
+        return false;
     }
 
-    dashes_c = g_array_sized_new (false, false, sizeof(double), len);
+    std::vector<double> dashes_c;
+    dashes_c.reserve(len);
     for (i = 0; i < len; ++i) {
         JS::Value elem;
         double b;
 
         elem = JS::UndefinedValue();
         if (!JS_GetElement(context, dashes, i, &elem)) {
-            goto out;
+            return false;
         }
         if (elem.isUndefined())
             continue;
 
         if (!JS::ToNumber(context, elem, &b))
-            goto out;
+            return false;
         if (b <= 0) {
             gjs_throw(context, "Dash value must be positive");
-            goto out;
+            return false;
         }
 
-        g_array_append_val(dashes_c, b);
+        dashes_c.push_back(b);
     }
 
     cr = gjs_cairo_context_get_context(context, obj);
-    cairo_set_dash(cr, (double*)dashes_c->data, dashes_c->len, offset);
+    cairo_set_dash(cr, &dashes_c[0], dashes_c.size(), offset);
     argv.rval().setUndefined();
-    retval = true;
- out:
-    if (dashes_c != NULL)
-        g_array_free (dashes_c, true);
-    return retval;
+    return true;
 }
 
 static JSBool