[gjs: 12/16] gtype: Fix error handling in gjs_gtype_get_actual_gtype()
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 12/16] gtype: Fix error handling in gjs_gtype_get_actual_gtype()
- Date: Sat, 3 Nov 2018 02:42:14 +0000 (UTC)
commit 221ffc189ad49b05e36aa38ca6cccee59c7dfd3e
Author: Philip Chimento <philip chimento gmail com>
Date: Sun Oct 28 20:12:32 2018 -0400
gtype: Fix error handling in gjs_gtype_get_actual_gtype()
This makes gjs_gtype_get_actual_gtype() conform to the JSAPI return
convention, so that it is clear when an exception is pending.
gi/arg.cpp | 44 ++++++++++++++++++++++++--------------------
gi/function.cpp | 3 ++-
gi/gtype.cpp | 42 ++++++++++++++++++++++--------------------
gi/gtype.h | 7 +++----
gi/private.cpp | 19 ++++++++++++++-----
gi/value.cpp | 54 +++++++++++++++++++++++++++++++-----------------------
6 files changed, 96 insertions(+), 73 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 170bb913..f1c0dcd9 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -790,11 +790,11 @@ gjs_gtypearray_to_array(JSContext *context,
unsigned int length,
void **arr_p)
{
- GType *result;
unsigned i;
/* add one so we're always zero terminated */
- result = (GType *) g_malloc0((length+1) * sizeof(GType));
+ GjsAutoPointer<GType, void, g_free> result =
+ static_cast<GType*>(g_malloc0((length + 1) * sizeof(GType)));
JS::RootedObject elem_obj(context), array(context, array_value.toObjectOrNull());
JS::RootedValue elem(context);
@@ -802,31 +802,28 @@ gjs_gtypearray_to_array(JSContext *context,
GType gtype;
elem = JS::UndefinedValue();
- if (!JS_GetElement(context, array, i, &elem)) {
- g_free(result);
- gjs_throw(context, "Missing array element %u", i);
+ if (!JS_GetElement(context, array, i, &elem))
return false;
- }
- if (!elem.isObjectOrNull())
- goto err;
+ if (!elem.isObject()) {
+ gjs_throw(context, "Invalid element in GType array");
+ return false;
+ }
- elem_obj = elem.toObjectOrNull();
- gtype = gjs_gtype_get_actual_gtype(context, elem_obj);
- if (gtype == G_TYPE_INVALID)
- goto err;
+ elem_obj = &elem.toObject();
+ if (!gjs_gtype_get_actual_gtype(context, elem_obj, >ype))
+ return false;
+ if (gtype == G_TYPE_INVALID) {
+ gjs_throw(context, "Invalid element in GType array");
+ return false;
+ }
result[i] = gtype;
}
- *arr_p = result;
+ *arr_p = result.release();
return true;
-
- err:
- g_free(result);
- gjs_throw(context, "Invalid element in GType array");
- return false;
}
GJS_JSAPI_RETURN_CONVENTION
@@ -1523,7 +1520,10 @@ gjs_value_to_g_argument(JSContext *context,
if (value.isObjectOrNull()) {
GType gtype;
JS::RootedObject obj(context, value.toObjectOrNull());
- gtype = gjs_gtype_get_actual_gtype(context, obj);
+ if (!gjs_gtype_get_actual_gtype(context, obj, >ype)) {
+ wrong = true;
+ break;
+ }
if (gtype == G_TYPE_INVALID)
wrong = true;
arg->v_ssize = gtype;
@@ -1679,7 +1679,11 @@ gjs_value_to_g_argument(JSContext *context,
GType actual_gtype;
gpointer klass;
- actual_gtype = gjs_gtype_get_actual_gtype(context, obj);
+ if (!gjs_gtype_get_actual_gtype(context, obj,
+ &actual_gtype)) {
+ wrong = true;
+ break;
+ }
if (actual_gtype == G_TYPE_NONE) {
wrong = true;
diff --git a/gi/function.cpp b/gi/function.cpp
index 82a00d56..2673eaf5 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -644,7 +644,8 @@ gjs_fill_method_instance(JSContext *context,
GType actual_gtype;
gpointer klass;
- actual_gtype = gjs_gtype_get_actual_gtype(context, obj);
+ if (!gjs_gtype_get_actual_gtype(context, obj, &actual_gtype))
+ return false;
if (actual_gtype == G_TYPE_NONE) {
gjs_throw(context, "Invalid GType class passed for instance parameter");
diff --git a/gi/gtype.cpp b/gi/gtype.cpp
index 2a46cfeb..c54bae55 100644
--- a/gi/gtype.cpp
+++ b/gi/gtype.cpp
@@ -185,41 +185,43 @@ gjs_gtype_create_gtype_wrapper (JSContext *context,
return *heap_wrapper;
}
-static GType
-_gjs_gtype_get_actual_gtype(JSContext *context,
- JS::HandleObject object,
- int recurse)
-{
- JSAutoRequest ar(context);
-
- if (JS_InstanceOf(context, object, &gjs_gtype_class, NULL))
- return GPOINTER_TO_SIZE(priv_from_js(context, object));
+GJS_JSAPI_RETURN_CONVENTION
+static bool _gjs_gtype_get_actual_gtype(JSContext* context,
+ JS::HandleObject object,
+ GType* gtype_out, int recurse) {
+ if (JS_InstanceOf(context, object, &gjs_gtype_class, nullptr)) {
+ *gtype_out = GPOINTER_TO_SIZE(priv_from_js(context, object));
+ return true;
+ }
JS::RootedValue gtype_val(context);
/* OK, we don't have a GType wrapper object -- grab the "$gtype"
* property on that and hope it's a GType wrapper object */
- if (!JS_GetProperty(context, object, "$gtype", >ype_val) ||
- !gtype_val.isObject()) {
-
+ if (!JS_GetProperty(context, object, "$gtype", >ype_val))
+ return false;
+ if (!gtype_val.isObject()) {
/* OK, so we're not a class. But maybe we're an instance. Check
for "constructor" and recurse on that. */
if (!JS_GetProperty(context, object, "constructor", >ype_val))
- return G_TYPE_INVALID;
+ return false;
}
if (recurse > 0 && gtype_val.isObject()) {
JS::RootedObject gtype_obj(context, >ype_val.toObject());
- return _gjs_gtype_get_actual_gtype(context, gtype_obj, recurse - 1);
+ return _gjs_gtype_get_actual_gtype(context, gtype_obj, gtype_out,
+ recurse - 1);
}
- return G_TYPE_INVALID;
+ *gtype_out = G_TYPE_INVALID;
+ return true;
}
-GType
-gjs_gtype_get_actual_gtype(JSContext *context,
- JS::HandleObject object)
-{
+bool gjs_gtype_get_actual_gtype(JSContext* context, JS::HandleObject object,
+ GType* gtype_out) {
+ g_assert(gtype_out && "Missing return location");
+ JSAutoRequest ar(context);
+
/* 2 means: recurse at most three times (including this
call).
The levels are calculated considering that, in the
@@ -228,7 +230,7 @@ gjs_gtype_get_actual_gtype(JSContext *context,
GType value.
*/
- return _gjs_gtype_get_actual_gtype(context, object, 2);
+ return _gjs_gtype_get_actual_gtype(context, object, gtype_out, 2);
}
bool
diff --git a/gi/gtype.h b/gi/gtype.h
index 3880b74f..d6f52582 100644
--- a/gi/gtype.h
+++ b/gi/gtype.h
@@ -38,10 +38,9 @@ GJS_JSAPI_RETURN_CONVENTION
JSObject * gjs_gtype_create_gtype_wrapper (JSContext *context,
GType gtype);
-/* FIXME: This should be GJS_JSAPI_RETURN_CONVENTION, but isn't */
-GJS_USE
-GType gjs_gtype_get_actual_gtype(JSContext *context,
- JS::HandleObject object);
+GJS_JSAPI_RETURN_CONVENTION
+bool gjs_gtype_get_actual_gtype(JSContext* context, JS::HandleObject object,
+ GType* gtype_out);
GJS_USE
bool gjs_typecheck_gtype (JSContext *context,
diff --git a/gi/private.cpp b/gi/private.cpp
index 6bdf5691..5c938e7d 100644
--- a/gi/private.cpp
+++ b/gi/private.cpp
@@ -51,7 +51,9 @@ static bool gjs_override_property(JSContext* cx, unsigned argc, JS::Value* vp) {
"type", &type))
return false;
- GType gtype = gjs_gtype_get_actual_gtype(cx, type);
+ GType gtype;
+ if (!gjs_gtype_get_actual_gtype(cx, type, >ype))
+ return false;
if (gtype == G_TYPE_INVALID) {
gjs_throw(cx, "Invalid parameter type was not a GType");
return false;
@@ -163,7 +165,9 @@ static bool get_interface_gtypes(JSContext* cx, JS::HandleObject interfaces,
}
JS::RootedObject iface(cx, &iface_val.toObject());
- GType iface_type = gjs_gtype_get_actual_gtype(cx, iface);
+ GType iface_type;
+ if (!gjs_gtype_get_actual_gtype(cx, iface, &iface_type))
+ return false;
if (iface_type == G_TYPE_INVALID) {
gjs_throw(
cx, "Invalid parameter interfaces (element %d was not a GType)",
@@ -354,7 +358,9 @@ static bool gjs_signal_new(JSContext* cx, unsigned argc, JS::Value* vp) {
accumulator = NULL;
}
- GType return_type = gjs_gtype_get_actual_gtype(cx, return_gtype_obj);
+ GType return_type;
+ if (!gjs_gtype_get_actual_gtype(cx, return_gtype_obj, &return_type))
+ return false;
if (accumulator == g_signal_accumulator_true_handled &&
return_type != G_TYPE_BOOLEAN) {
@@ -378,10 +384,13 @@ static bool gjs_signal_new(JSContext* cx, unsigned argc, JS::Value* vp) {
}
JS::RootedObject gjs_gtype(cx, >ype_val.toObject());
- params[ix] = gjs_gtype_get_actual_gtype(cx, gjs_gtype);
+ if (!gjs_gtype_get_actual_gtype(cx, gjs_gtype, ¶ms[ix]))
+ return false;
}
- GType gtype = gjs_gtype_get_actual_gtype(cx, gtype_obj);
+ GType gtype;
+ if (!gjs_gtype_get_actual_gtype(cx, gtype_obj, >ype))
+ return false;
unsigned signal_id = g_signal_newv(
signal_name.get(), gtype, GSignalFlags(flags),
diff --git a/gi/value.cpp b/gi/value.cpp
index 3467132b..fabecca2 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -321,32 +321,38 @@ gjs_closure_new_marshaled (JSContext *context,
return closure;
}
-GJS_USE
-static GType
-gjs_value_guess_g_type(JSContext *context,
- JS::Value value)
-{
- if (value.isNull())
- return G_TYPE_POINTER;
-
- if (value.isString())
- return G_TYPE_STRING;
-
- if (value.isInt32())
- return G_TYPE_INT;
-
- if (value.isDouble())
- return G_TYPE_DOUBLE;
-
- if (value.isBoolean())
- return G_TYPE_BOOLEAN;
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_value_guess_g_type(JSContext* context, JS::Value value,
+ GType* gtype_out) {
+ g_assert(gtype_out && "Invalid return location");
+ if (value.isNull()) {
+ *gtype_out = G_TYPE_POINTER;
+ return true;
+ }
+ if (value.isString()) {
+ *gtype_out = G_TYPE_STRING;
+ return true;
+ }
+ if (value.isInt32()) {
+ *gtype_out = G_TYPE_INT;
+ return true;
+ }
+ if (value.isDouble()) {
+ *gtype_out = G_TYPE_DOUBLE;
+ return true;
+ }
+ if (value.isBoolean()) {
+ *gtype_out = G_TYPE_BOOLEAN;
+ return true;
+ }
if (value.isObject()) {
JS::RootedObject obj(context, &value.toObject());
- return gjs_gtype_get_actual_gtype(context, obj);
+ return gjs_gtype_get_actual_gtype(context, obj, gtype_out);
}
- return G_TYPE_INVALID;
+ *gtype_out = G_TYPE_INVALID;
+ return true;
}
static bool
@@ -374,7 +380,8 @@ gjs_value_to_g_value_internal(JSContext *context,
gtype = G_VALUE_TYPE(gvalue);
if (gtype == 0) {
- gtype = gjs_value_guess_g_type(context, value);
+ if (!gjs_value_guess_g_type(context, value, >ype))
+ return false;
if (gtype == G_TYPE_INVALID) {
gjs_throw(context, "Could not guess unspecified GValue type");
@@ -658,7 +665,8 @@ gjs_value_to_g_value_internal(JSContext *context,
return throw_expect_type(context, value, "GType object");
JS::RootedObject obj(context, &value.toObject());
- type = gjs_gtype_get_actual_gtype(context, obj);
+ if (!gjs_gtype_get_actual_gtype(context, obj, &type))
+ return false;
g_value_set_gtype(gvalue, type);
} else if (g_type_is_a(gtype, G_TYPE_POINTER)) {
if (value.isNull()) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]