[gjs/wip/ptomato/warnings: 5/10] js: Use std::vector instead of GArray
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/ptomato/warnings: 5/10] js: Use std::vector instead of GArray
- Date: Sun, 2 Oct 2016 23:12:43 +0000 (UTC)
commit fda4125859ed0e6560c02f4042bce69b453bf8b2
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 | 34 ++++++++++++++--------------------
3 files changed, 30 insertions(+), 53 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index d739f01..662eb53 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -24,6 +24,7 @@
#include <config.h>
#include <string.h>
+#include <vector>
#include <gjs/gi.h>
#include "object.h"
@@ -658,7 +659,6 @@ free_g_params(GParameter *params,
for (i = 0; i < n_params; ++i) {
g_value_unset(¶ms[i].value);
}
- g_free(params);
}
/* Set properties from args to constructor (argv[0] is supposed to be
@@ -670,24 +670,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");
@@ -735,29 +725,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;
}
@@ -1247,8 +1225,7 @@ object_instance_init (JSContext *context,
{
ObjectInstance *priv;
GType gtype;
- GParameter *params;
- int n_params;
+ std::vector<GParameter> params;
GTypeQuery query;
JSObject *old_jsobj;
GObject *gobj;
@@ -1259,8 +1236,7 @@ object_instance_init (JSContext *context,
g_assert(gtype != G_TYPE_NONE);
if (!object_instance_props_to_g_parameters(context, *object, argc, argv,
- gtype,
- ¶ms, &n_params)) {
+ gtype, params)) {
return false;
}
@@ -1271,9 +1247,9 @@ object_instance_init (JSContext *context,
if (g_type_get_qdata(gtype, gjs_is_custom_type_quark()))
object_init_list = g_slist_prepend(object_init_list, *object);
- gobj = (GObject*) g_object_newv(gtype, n_params, params);
+ gobj = (GObject*) g_object_newv(gtype, params.size(), ¶ms[0]);
- free_g_params(params, n_params);
+ free_g_params(¶ms[0], params.size());
old_jsobj = 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 d3e5787..f1d4282 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 8fca420..0e55823 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>
@@ -566,59 +568,51 @@ setDash_func(JSContext *context,
guint i;
cairo_t *cr;
- JSObject *dashes;
+ 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, "offset", &offset))
+ "dashes", dashes.address(), "offset", &offset))
return false;
- JS_AddObjectRoot(context, &dashes);
-
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_ValueToNumber(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);
- JS_RemoveObjectRoot(context, &dashes);
- return retval;
+ return true;
}
static bool
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]