[gjs: 4/16] js: Fix missing error checks
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 4/16] js: Fix missing error checks
- Date: Sat, 3 Nov 2018 02:41:34 +0000 (UTC)
commit 5aa46c2ef4e8c26864d33b4a042cf9c021a7099a
Author: Philip Chimento <philip chimento gmail com>
Date: Sun Oct 28 15:57:31 2018 -0400
js: Fix missing error checks
These fixes were caught by the compiler warnings added in the subsequent
annotations commit. They are placed here in a separate commit for ease
of review.
gi/arg.cpp | 14 +++++---------
gi/boxed.cpp | 26 ++++++++++++++++----------
gi/enumeration.cpp | 17 +++++++++--------
gi/function.cpp | 16 +++++++---------
gi/fundamental.cpp | 8 ++++++--
gi/gerror.cpp | 26 ++++++++++----------------
gi/interface.cpp | 26 +++++++++++++++-----------
gi/object.cpp | 7 ++++++-
gi/private.cpp | 10 ++++++----
gi/repo.cpp | 8 ++++----
gi/union.cpp | 18 ++++++++++--------
gjs/context.cpp | 12 ++++++++++--
gjs/jsapi-class.h | 4 ++--
test/gjs-test-coverage.cpp | 7 +++----
test/gjs-tests.cpp | 3 ++-
15 files changed, 111 insertions(+), 91 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index be1237d0..cb282ff5 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -3445,11 +3445,9 @@ gjs_g_arg_release_internal(JSContext *context,
GArgument arg_iter;
arg_iter.v_pointer = g_array_index (array, gpointer, i);
- gjs_g_arg_release_internal(context,
- transfer,
- param_info,
- element_type,
- &arg_iter);
+ failed = !gjs_g_arg_release_internal(
+ context, transfer, param_info, element_type,
+ &arg_iter);
}
g_array_free (array, true);
@@ -3482,10 +3480,8 @@ gjs_g_arg_release_internal(JSContext *context,
GArgument arg_iter;
arg_iter.v_pointer = g_ptr_array_index (array, i);
- gjs_g_argument_release(context,
- transfer,
- param_info,
- &arg_iter);
+ failed = !gjs_g_argument_release(context, transfer,
+ param_info, &arg_iter);
}
}
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index c430c877..30851291 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -89,10 +89,9 @@ gjs_define_static_methods(JSContext *context,
n_methods = g_struct_info_get_n_methods(boxed_info);
for (i = 0; i < n_methods; i++) {
- GIFunctionInfo *meth_info;
GIFunctionInfoFlags flags;
- meth_info = g_struct_info_get_method (boxed_info, i);
+ GjsAutoFunctionInfo meth_info = g_struct_info_get_method(boxed_info, i);
flags = g_function_info_get_flags (meth_info);
/* Anything that isn't a method we put on the prototype of the
@@ -103,11 +102,9 @@ gjs_define_static_methods(JSContext *context,
* like in the near future.
*/
if (!(flags & GI_FUNCTION_IS_METHOD)) {
- gjs_define_function(context, constructor, gtype,
- (GICallableInfo *)meth_info);
+ if (!gjs_define_function(context, constructor, gtype, meth_info))
+ return false;
}
-
- g_base_info_unref((GIBaseInfo*) meth_info);
}
return true;
}
@@ -562,6 +559,8 @@ get_nested_interface_object(JSContext *context,
JS::RootedObject proto(context,
gjs_lookup_generic_prototype(context,
(GIBoxedInfo*) interface_info));
+ if (!proto)
+ return false;
proto_priv = priv_from_js(context, proto);
offset = g_field_info_get_offset (field_info);
@@ -682,6 +681,8 @@ set_nested_interface_object (JSContext *context,
JS::RootedObject proto(context,
gjs_lookup_generic_prototype(context,
(GIBoxedInfo*) interface_info));
+ if (!proto)
+ return false;
proto_priv = priv_from_js(context, proto);
/* If we can't directly copy from the source object we need
@@ -762,10 +763,13 @@ boxed_set_field_from_value(JSContext *context,
success = true;
out:
- if (need_release)
- gjs_g_argument_release (context, GI_TRANSFER_NOTHING,
- type_info,
- &arg);
+ if (need_release) {
+ JS::AutoSaveExceptionState saved_exc(context);
+ if (!gjs_g_argument_release(context, GI_TRANSFER_NOTHING, type_info,
+ &arg))
+ gjs_log_exception(context);
+ saved_exc.restore();
+ }
g_base_info_unref ((GIBaseInfo *)type_info);
@@ -1173,6 +1177,8 @@ gjs_boxed_from_c_struct(JSContext *context,
g_base_info_get_name((GIBaseInfo *)info), gboxed);
JS::RootedObject proto(context, gjs_lookup_generic_prototype(context, info));
+ if (!proto)
+ return nullptr;
proto_priv = priv_from_js(context, proto);
obj = JS_NewObjectWithGivenProto(context, JS_GetClass(proto), proto);
diff --git a/gi/enumeration.cpp b/gi/enumeration.cpp
index 9f6c91f9..ade2295b 100644
--- a/gi/enumeration.cpp
+++ b/gi/enumeration.cpp
@@ -106,6 +106,9 @@ gjs_define_enum_values(JSContext *context,
gtype = g_registered_type_info_get_g_type((GIRegisteredTypeInfo*)info);
JS::RootedObject gtype_obj(context,
gjs_gtype_create_gtype_wrapper(context, gtype));
+ if (!gtype_obj)
+ return false;
+
JS_DefineProperty(context, in_object, "$gtype", gtype_obj, JSPROP_PERMANENT);
return true;
@@ -121,10 +124,9 @@ gjs_define_enum_static_methods(JSContext *context,
n_methods = g_enum_info_get_n_methods(enum_info);
for (i = 0; i < n_methods; i++) {
- GIFunctionInfo *meth_info;
GIFunctionInfoFlags flags;
- meth_info = g_enum_info_get_method(enum_info, i);
+ GjsAutoFunctionInfo meth_info = g_enum_info_get_method(enum_info, i);
flags = g_function_info_get_flags(meth_info);
g_warn_if_fail(!(flags & GI_FUNCTION_IS_METHOD));
@@ -136,11 +138,10 @@ gjs_define_enum_static_methods(JSContext *context,
* like in the near future.
*/
if (!(flags & GI_FUNCTION_IS_METHOD)) {
- gjs_define_function(context, constructor, G_TYPE_NONE,
- (GICallableInfo *)meth_info);
+ if (!gjs_define_function(context, constructor, G_TYPE_NONE,
+ meth_info))
+ return false;
}
-
- g_base_info_unref((GIBaseInfo*) meth_info);
}
return true;
@@ -171,9 +172,9 @@ gjs_define_enumeration(JSContext *context,
enum_name);
}
- if (!gjs_define_enum_values(context, enum_obj, info))
+ if (!gjs_define_enum_values(context, enum_obj, info) ||
+ !gjs_define_enum_static_methods(context, enum_obj, info))
return false;
- gjs_define_enum_static_methods (context, enum_obj, info);
gjs_debug(GJS_DEBUG_GENUM,
"Defining %s.%s as %p",
diff --git a/gi/function.cpp b/gi/function.cpp
index 0491187b..49f14eef 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -1303,16 +1303,14 @@ release:
transfer = g_arg_info_get_ownership_transfer(&arg_info);
if (!arg_failed) {
if (array_length_pos >= 0) {
- gjs_g_argument_release_out_array(context,
- transfer,
- &arg_type_info,
- array_length.toInt32(),
- arg);
+ if (!gjs_g_argument_release_out_array(
+ context, transfer, &arg_type_info,
+ array_length.toInt32(), arg))
+ postinvoke_release_failed = true;
} else {
- gjs_g_argument_release(context,
- transfer,
- &arg_type_info,
- arg);
+ if (!gjs_g_argument_release(context, transfer,
+ &arg_type_info, arg))
+ postinvoke_release_failed = true;
}
}
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 6769b188..c661d0d0 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -573,8 +573,9 @@ gjs_lookup_fundamental_prototype(JSContext *context,
we need to define it first.
*/
JS::RootedObject ignored(context);
- gjs_define_fundamental_class(context, in_object, info, &constructor,
- &ignored);
+ if (!gjs_define_fundamental_class(context, in_object, info,
+ &constructor, &ignored))
+ return nullptr;
} else {
if (G_UNLIKELY (!value.isObject()))
return NULL;
@@ -701,6 +702,9 @@ gjs_define_fundamental_class(JSContext *context,
JS::RootedObject gtype_obj(context,
gjs_gtype_create_gtype_wrapper(context, gtype));
+ if (!gtype_obj)
+ return false;
+
return JS_DefineProperty(context, constructor, "$gtype", gtype_obj,
JSPROP_PERMANENT);
}
diff --git a/gi/gerror.cpp b/gi/gerror.cpp
index 28ac547f..5fadd83c 100644
--- a/gi/gerror.cpp
+++ b/gi/gerror.cpp
@@ -325,22 +325,16 @@ gjs_define_error_class(JSContext *context,
gjs_lookup_generic_prototype(context, glib_error_info));
g_base_info_unref((GIBaseInfo*)glib_error_info);
- if (!gjs_init_class_dynamic(context, in_object,
- parent_proto,
- g_base_info_get_namespace( (GIBaseInfo*) info),
- constructor_name,
- &gjs_error_class,
- gjs_error_constructor, 1,
- /* props of prototype */
- &gjs_error_proto_props[0],
- /* funcs of prototype */
- &gjs_error_proto_funcs[0],
- /* props of constructor, MyConstructor.myprop */
- NULL,
- /* funcs of constructor, MyConstructor.myfunc() */
- &gjs_error_constructor_funcs[0],
- &prototype,
- &constructor)) {
+ if (!parent_proto ||
+ !gjs_init_class_dynamic(
+ context, in_object, parent_proto, g_base_info_get_namespace(info),
+ constructor_name, &gjs_error_class, gjs_error_constructor, 1,
+ gjs_error_proto_props, // props of prototype
+ gjs_error_proto_funcs, // funcs of prototype
+ nullptr, // props of constructor, MyConstructor.myprop
+ gjs_error_constructor_funcs, // funcs of constructor,
+ // MyConstructor.myfunc()
+ &prototype, &constructor)) {
gjs_log_exception(context);
g_error("Can't init class %s", constructor_name);
}
diff --git a/gi/interface.cpp b/gi/interface.cpp
index ac7467da..266689bf 100644
--- a/gi/interface.cpp
+++ b/gi/interface.cpp
@@ -83,10 +83,9 @@ gjs_define_static_methods(JSContext *context,
n_methods = g_interface_info_get_n_methods(info);
for (i = 0; i < n_methods; i++) {
- GIFunctionInfo *meth_info;
GIFunctionInfoFlags flags;
- meth_info = g_interface_info_get_method (info, i);
+ GjsAutoFunctionInfo meth_info = g_interface_info_get_method(info, i);
flags = g_function_info_get_flags (meth_info);
/* Anything that isn't a method we put on the prototype of the
@@ -97,11 +96,9 @@ gjs_define_static_methods(JSContext *context,
* like in the near future.
*/
if (!(flags & GI_FUNCTION_IS_METHOD)) {
- gjs_define_function(context, constructor, gtype,
- (GICallableInfo *)meth_info);
+ if (!gjs_define_function(context, constructor, gtype, meth_info))
+ return false;
}
-
- g_base_info_unref((GIBaseInfo*) meth_info);
}
return true;
}
@@ -162,9 +159,10 @@ interface_has_instance_func(JSContext *cx,
g_assert(interface.isObject());
JS::RootedObject interface_constructor(cx, &interface.toObject());
JS::RootedObject interface_proto(cx);
- gjs_object_require_property(cx, interface_constructor,
- "interface constructor",
- GJS_STRING_PROTOTYPE, &interface_proto);
+ if (!gjs_object_require_property(cx, interface_constructor,
+ "interface constructor",
+ GJS_STRING_PROTOTYPE, &interface_proto))
+ return false;
Interface *priv;
if (!priv_from_js_with_typecheck(cx, interface_proto, &priv))
@@ -247,11 +245,17 @@ gjs_define_interface_class(JSContext *context,
/* If we have no GIRepository information, then this interface was defined
* from within GJS and therefore has no C static methods to be defined. */
- if (priv->info)
- gjs_define_static_methods(context, constructor, priv->gtype, priv->info);
+ if (priv->info) {
+ if (!gjs_define_static_methods(context, constructor, priv->gtype,
+ priv->info))
+ return false;
+ }
JS::RootedObject gtype_obj(context,
gjs_gtype_create_gtype_wrapper(context, priv->gtype));
+ if (!gtype_obj)
+ return false;
+
JS_DefineProperty(context, constructor, "$gtype", gtype_obj, JSPROP_PERMANENT);
return true;
diff --git a/gi/object.cpp b/gi/object.cpp
index 8265cd2e..530561d5 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -875,7 +875,9 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
return true;
}
- gjs_define_function(context, obj, m_gtype, vfunc);
+ if (!gjs_define_function(context, obj, m_gtype, vfunc))
+ return false;
+
*resolved = true;
return true;
}
@@ -2240,6 +2242,9 @@ gjs_define_object_class(JSContext *context,
JS::RootedObject gtype_obj(context,
gjs_gtype_create_gtype_wrapper(context, gtype));
+ if (!gtype_obj)
+ return false;
+
return JS_DefineProperty(context, constructor, "$gtype", gtype_obj,
JSPROP_PERMANENT);
}
diff --git a/gi/private.cpp b/gi/private.cpp
index 097b8a6f..64bacbf5 100644
--- a/gi/private.cpp
+++ b/gi/private.cpp
@@ -220,8 +220,9 @@ static bool gjs_register_interface(JSContext* cx, unsigned argc,
return false; // error will have been thrown already
JS::RootedObject constructor(cx);
- gjs_define_interface_class(cx, module, nullptr, interface_type,
- &constructor);
+ if (!gjs_define_interface_class(cx, module, nullptr, interface_type,
+ &constructor))
+ return false;
args.rval().setObject(*constructor);
return true;
@@ -302,8 +303,9 @@ static bool gjs_register_type(JSContext* cx, unsigned argc, JS::Value* vp) {
/* create a custom JSClass */
JS::RootedObject module(cx, gjs_lookup_private_namespace(cx));
JS::RootedObject constructor(cx), prototype(cx);
- gjs_define_object_class(cx, module, nullptr, instance_type, &constructor,
- &prototype);
+ if (!gjs_define_object_class(cx, module, nullptr, instance_type,
+ &constructor, &prototype))
+ return false;
auto* priv = ObjectPrototype::for_js(cx, prototype);
priv->set_type_qdata();
diff --git a/gi/repo.cpp b/gi/repo.cpp
index 495c327c..2a3d110a 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -479,10 +479,10 @@ gjs_define_info(JSContext *context,
case GI_INFO_TYPE_INTERFACE:
{
JS::RootedObject ignored(context);
- gjs_define_interface_class(context, in_object,
- (GIInterfaceInfo *) info,
- g_registered_type_info_get_g_type((GIRegisteredTypeInfo *) info),
- &ignored);
+ if (!gjs_define_interface_class(
+ context, in_object, info,
+ g_registered_type_info_get_g_type(info), &ignored))
+ return false;
}
break;
case GI_INFO_TYPE_INVALID:
diff --git a/gi/union.cpp b/gi/union.cpp
index 853c908a..13f1a80e 100644
--- a/gi/union.cpp
+++ b/gi/union.cpp
@@ -130,10 +130,9 @@ union_new(JSContext *context,
n_methods = g_union_info_get_n_methods(info);
for (i = 0; i < n_methods; ++i) {
- GIFunctionInfo *func_info;
GIFunctionInfoFlags flags;
- func_info = g_union_info_get_method(info, i);
+ GjsAutoFunctionInfo func_info = g_union_info_get_method(info, i);
flags = g_function_info_get_flags(func_info);
if ((flags & GI_FUNCTION_IS_CONSTRUCTOR) != 0 &&
@@ -141,10 +140,10 @@ union_new(JSContext *context,
JS::RootedValue rval(context, JS::NullValue());
- gjs_invoke_c_function_uncached(context, func_info, obj,
- JS::HandleValueArray::empty(), &rval);
-
- g_base_info_unref((GIBaseInfo*) func_info);
+ if (!gjs_invoke_c_function_uncached(context, func_info, obj,
+ JS::HandleValueArray::empty(),
+ &rval))
+ return nullptr;
/* We are somewhat wasteful here; invoke_c_function() above
* creates a JSObject wrapper for the union that we immediately
@@ -157,8 +156,6 @@ union_new(JSContext *context,
return gjs_c_union_from_union(context, rval_obj);
}
}
-
- g_base_info_unref((GIBaseInfo*) func_info);
}
gjs_throw(context, "Unable to construct union type %s since it has no zero-args <constructor>, can only
wrap an existing one",
@@ -355,6 +352,9 @@ gjs_define_union_class(JSContext *context,
JS::RootedObject gtype_obj(context,
gjs_gtype_create_gtype_wrapper(context, gtype));
+ if (!gtype_obj)
+ return false;
+
JS_DefineProperty(context, constructor, "$gtype", gtype_obj, JSPROP_PERMANENT);
return true;
@@ -387,6 +387,8 @@ gjs_union_from_c_union(JSContext *context,
JS::RootedObject proto(context,
gjs_lookup_generic_prototype(context, (GIUnionInfo*) info));
+ if (!proto)
+ return nullptr;
obj = JS_NewObjectWithGivenProto(context, JS_GetClass(proto), proto);
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 1af42788..df6c487e 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -702,7 +702,11 @@ static gboolean
drain_job_queue_idle_handler(void *data)
{
auto gjs_context = static_cast<GjsContext *>(data);
- _gjs_context_run_jobs(gjs_context);
+ if (!_gjs_context_run_jobs(gjs_context)) {
+ auto* cx = static_cast<JSContext*>(
+ gjs_context_get_native_context(gjs_context));
+ gjs_log_exception(cx);
+ }
/* Uncatchable exceptions are swallowed here - no way to get a handle on
* the main loop to exit it from this idle handler */
g_assert(((void) "_gjs_context_run_jobs() should have emptied queue",
@@ -720,8 +724,12 @@ _gjs_context_enqueue_job(GjsContext *gjs_context,
else
g_assert(gjs_context->job_queue->length() == 0);
- if (!gjs_context->job_queue->append(job))
+ if (!gjs_context->job_queue->append(job)) {
+ auto* cx = static_cast<JSContext*>(
+ gjs_context_get_native_context(gjs_context));
+ JS_ReportOutOfMemory(cx);
return false;
+ }
if (!gjs_context->idle_drain_handler)
gjs_context->idle_drain_handler =
g_idle_add_full(G_PRIORITY_DEFAULT, drain_job_queue_idle_handler,
diff --git a/gjs/jsapi-class.h b/gjs/jsapi-class.h
index 0473e12b..32f8d315 100644
--- a/gjs/jsapi-class.h
+++ b/gjs/jsapi-class.h
@@ -279,8 +279,8 @@ gjs_##cname##_define_proto(JSContext *cx, \
if (gtype != G_TYPE_NONE) { \
JS::RootedObject gtype_obj(cx, \
gjs_gtype_create_gtype_wrapper(cx, gtype)); \
- if (!JS_DefineProperty(cx, ctor_obj, "$gtype", gtype_obj, \
- JSPROP_PERMANENT)) \
+ if (!gtype_obj || !JS_DefineProperty(cx, ctor_obj, "$gtype", \
+ gtype_obj, JSPROP_PERMANENT)) \
return false; \
} \
gjs_debug(GJS_DEBUG_CONTEXT, "Initialized class %s prototype %p", \
diff --git a/test/gjs-test-coverage.cpp b/test/gjs-test-coverage.cpp
index 7d3cbcdb..996bcb94 100644
--- a/test/gjs-test-coverage.cpp
+++ b/test/gjs-test-coverage.cpp
@@ -305,10 +305,9 @@ test_covered_file_is_duplicated_into_output_if_resource(gpointer fixture_da
fixture->coverage = gjs_coverage_new(coverage_scripts, fixture->context,
fixture->lcov_output_dir);
- gjs_context_eval_file(fixture->context,
- mock_resource_filename,
- NULL,
- NULL);
+ bool ok = gjs_context_eval_file(fixture->context, mock_resource_filename,
+ nullptr, nullptr);
+ g_assert_true(ok);
gjs_coverage_write_statistics(fixture->coverage);
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 328c61c2..309270c6 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -149,7 +149,8 @@ gjstest_test_func_gjs_jsapi_util_error_throw(GjsUnitTestFixture *fx,
g_assert(value.isString());
JS::UniqueChars s;
- gjs_string_to_utf8(fx->cx, value, &s);
+ bool ok = gjs_string_to_utf8(fx->cx, value, &s);
+ g_assert_true(ok);
g_assert_nonnull(s);
g_assert_cmpstr(s.get(), ==, "This is an exception 42");
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
