[gjs] object: Avoid taking address of empty vector



commit ac42789ad669ec5acd617b57def46fbb4328a746
Author: Philip Chimento <philip chimento gmail com>
Date:   Tue Aug 29 23:20:41 2017 -0700

    object: Avoid taking address of empty vector
    
    Given a std::vector<T> vec, &vec[0] is undefined behaviour when the
    vector is empty. C++11 provides a vec.data() method which does the same
    thing safely.
    
    This is an opportunity for a small refactor; the free function didn't
    actually free the vector, so it should be renamed "clear", and it can
    just take in the vector instead of a pointer to the data. We can also
    keep all the memory management of the vector in the same place, in
    object_instance_init().
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786995

 gi/object.cpp |   44 ++++++++++++++++++--------------------------
 1 files changed, 18 insertions(+), 26 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index a072242..700afa3 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -876,19 +876,10 @@ object_instance_resolve(JSContext       *context,
     return true;
 }
 
-static void
-free_g_params(GParameter *params,
-              int         n_params)
-{
-    int i;
-
-    for (i = 0; i < n_params; ++i) {
-        g_value_unset(&params[i].value);
-    }
-}
-
 /* Set properties from args to constructor (argv[0] is supposed to be
  * a hash)
+ * The GParameter elements in the passed-in vector must be unset by the caller,
+ * regardless of the return value of this function.
  */
 static bool
 object_instance_props_to_g_parameters(JSContext                  *context,
@@ -904,7 +895,6 @@ object_instance_props_to_g_parameters(JSContext                  *context,
 
     if (!args[0].isObject()) {
         gjs_throw(context, "argument should be a hash with props to set");
-        free_g_params(&gparams[0], gparams.size());
         return false;
     }
 
@@ -914,7 +904,7 @@ object_instance_props_to_g_parameters(JSContext                  *context,
     JS::Rooted<JS::IdVector> ids(context, context);
     if (!JS_Enumerate(context, props, &ids)) {
         gjs_throw(context, "Failed to create property iterator for object props hash");
-        goto free_array_and_fail;
+        return false;
     }
 
     for (ix = 0, length = ids.length(); ix < length; ix++) {
@@ -925,12 +915,10 @@ object_instance_props_to_g_parameters(JSContext                  *context,
          * doesn't know that */
         prop_id = ids[ix];
 
-        if (!gjs_object_require_property(context, props, "property list", prop_id, &value)) {
-            goto free_array_and_fail;
-        }
-
-        if (!gjs_get_string_id(context, prop_id, &name))
-            goto free_array_and_fail;
+        if (!gjs_object_require_property(context, props, "property list",
+                                         prop_id, &value) ||
+            !gjs_get_string_id(context, prop_id, &name))
+            return false;
 
         switch (init_g_param_from_property(context, name,
                                            value,
@@ -942,7 +930,7 @@ object_instance_props_to_g_parameters(JSContext                  *context,
                       name.get(), g_type_name(gtype));
             /* fallthrough */
         case SOME_ERROR_OCCURRED:
-            goto free_array_and_fail;
+            return false;
         case VALUE_WAS_SET:
         default:
             break;
@@ -952,10 +940,6 @@ object_instance_props_to_g_parameters(JSContext                  *context,
     }
 
     return true;
-
- free_array_and_fail:
-    free_g_params(&gparams[0], gparams.size());
-    return false;
 }
 
 #define DEBUG_DISPOSE 0
@@ -1324,6 +1308,13 @@ disassociate_js_gobject(GObject *gobj)
     priv->js_object_finalized = true;
 }
 
+static void
+clear_g_params(std::vector<GParameter>& params)
+{
+    for (GParameter param : params)
+        g_value_unset(&param.value);
+}
+
 static bool
 object_instance_init (JSContext                  *context,
                       JS::MutableHandleObject     object,
@@ -1342,6 +1333,7 @@ object_instance_init (JSContext                  *context,
 
     if (!object_instance_props_to_g_parameters(context, object, args,
                                                gtype, params)) {
+        clear_g_params(params);
         return false;
     }
 
@@ -1354,10 +1346,10 @@ object_instance_init (JSContext                  *context,
     }
 
 G_GNUC_BEGIN_IGNORE_DEPRECATIONS
-    gobj = (GObject*) g_object_newv(gtype, params.size(), &params[0]);
+    gobj = (GObject*) g_object_newv(gtype, params.size(), params.data());
 G_GNUC_END_IGNORE_DEPRECATIONS
 
-    free_g_params(&params[0], params.size());
+    clear_g_params(params);
 
     ObjectInstance *other_priv = get_object_qdata(gobj);
     if (other_priv && other_priv->keep_alive != object.get()) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]