[gjs: 1/12] function: Save original allocated pointers

[Philip Chimento <pchimento src gnome org>]

commit da3c0f2b1ac7b52339e12adc92fbf5e968c4eb6e
Author: Philip Chimento <philip chimento gmail com>
Date:   Fri Mar 12 11:20:05 2021 -0800

    function: Save original allocated pointers
    
    cppcheck is not smart enough to determine that when we save an allocated
    pointer plus an offset, and then free the saved pointer minus that offset,
    it amounts to the same thing. But, maybe it is clearer anyway to save the
    original pointers and add the offset in an accessor.

 gi/arg-cache.cpp | 57 +++++++++++++++++++++++++++-----------------------------
 gi/arg-cache.h   |  2 +-
 gi/function.cpp  | 24 ++++++++++++------------
 gi/function.h    | 34 +++++++++++++++++++++++----------
 4 files changed, 64 insertions(+), 53 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 1598dd02..10b15fcc 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -201,8 +201,8 @@ static bool gjs_marshal_generic_inout_in(JSContext* cx, GjsArgumentCache* self,
         return false;
 
     int ix = self->arg_pos;
-    state->out_cvalues[ix] = state->inout_original_cvalues[ix] = *arg;
-    gjs_arg_set(arg, &state->out_cvalues[ix]);
+    state->out_cvalue(ix) = state->inout_original_cvalue(ix) = *arg;
+    gjs_arg_set(arg, &state->out_cvalue(ix));
     return true;
 }
 
@@ -222,7 +222,7 @@ static bool gjs_marshal_explicit_array_in_in(JSContext* cx,
 
     uint8_t length_pos = self->contents.array.length_pos;
     gjs_g_argument_set_array_length(self->contents.array.length_tag,
-                                    &state->in_cvalues[length_pos], length);
+                                    &state->in_cvalue(length_pos), length);
     gjs_arg_set(arg, data);
     return true;
 }
@@ -242,21 +242,21 @@ static bool gjs_marshal_explicit_array_inout_in(JSContext* cx,
     if (!gjs_arg_get<void*>(arg)) {
         // Special case where we were given JS null to also pass null for
         // length, and not a pointer to an integer that derefs to 0.
-        gjs_arg_unset<void*>(&state->in_cvalues[length_pos]);
-        gjs_arg_unset<int>(&state->out_cvalues[length_pos]);
-        gjs_arg_unset<int>(&state->inout_original_cvalues[length_pos]);
+        gjs_arg_unset<void*>(&state->in_cvalue(length_pos));
+        gjs_arg_unset<int>(&state->out_cvalue(length_pos));
+        gjs_arg_unset<int>(&state->inout_original_cvalue(length_pos));
 
-        gjs_arg_unset<void*>(&state->out_cvalues[ix]);
-        gjs_arg_unset<void*>(&state->inout_original_cvalues[ix]);
+        gjs_arg_unset<void*>(&state->out_cvalue(ix));
+        gjs_arg_unset<void*>(&state->inout_original_cvalue(ix));
     } else {
-        state->out_cvalues[length_pos] =
-            state->inout_original_cvalues[length_pos] =
-                state->in_cvalues[length_pos];
-        gjs_arg_set(&state->in_cvalues[length_pos],
-                    &state->out_cvalues[length_pos]);
-
-        state->out_cvalues[ix] = state->inout_original_cvalues[ix] = *arg;
-        gjs_arg_set(arg, &state->out_cvalues[ix]);
+        state->out_cvalue(length_pos) =
+            state->inout_original_cvalue(length_pos) =
+                state->in_cvalue(length_pos);
+        gjs_arg_set(&state->in_cvalue(length_pos),
+                    &state->out_cvalue(length_pos));
+
+        state->out_cvalue(ix) = state->inout_original_cvalue(ix) = *arg;
+        gjs_arg_set(arg, &state->out_cvalue(ix));
     }
 
     return true;
@@ -313,11 +313,11 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
                     static_cast<GjsCallbackTrampoline*>(data));
             };
         }
-        gjs_arg_set(&state->in_cvalues[destroy_pos], destroy_notify);
+        gjs_arg_set(&state->in_cvalue(destroy_pos), destroy_notify);
     }
     if (self->has_callback_closure()) {
         uint8_t closure_pos = self->contents.callback.closure_pos;
-        gjs_arg_set(&state->in_cvalues[closure_pos], trampoline);
+        gjs_arg_set(&state->in_cvalue(closure_pos), trampoline);
     }
 
     if (trampoline && self->contents.callback.scope == GI_SCOPE_TYPE_ASYNC) {
@@ -335,9 +335,8 @@ static bool gjs_marshal_generic_out_in(JSContext*, GjsArgumentCache* self,
                                        GjsFunctionCallState* state,
                                        GIArgument* arg, JS::HandleValue) {
     // Default value in case a broken C function doesn't fill in the pointer
-    gjs_arg_unset<void*>(&state->out_cvalues[self->arg_pos]);
-    gjs_arg_set(arg,
-                &gjs_arg_member<void*>(&state->out_cvalues[self->arg_pos]));
+    gjs_arg_unset<void*>(&state->out_cvalue(self->arg_pos));
+    gjs_arg_set(arg, &gjs_arg_member<void*>(&state->out_cvalue(self->arg_pos)));
     return true;
 }
 
@@ -347,7 +346,7 @@ static bool gjs_marshal_caller_allocates_in(JSContext*, GjsArgumentCache* self,
                                             GIArgument* arg, JS::HandleValue) {
     void* blob = g_malloc0(self->contents.caller_allocates_size);
     gjs_arg_set(arg, blob);
-    gjs_arg_set(&state->out_cvalues[self->arg_pos], blob);
+    gjs_arg_set(&state->out_cvalue(self->arg_pos), blob);
     return true;
 }
 
@@ -804,7 +803,7 @@ static bool gjs_marshal_explicit_array_out_out(JSContext* cx,
                                                GIArgument* arg,
                                                JS::MutableHandleValue value) {
     uint8_t length_pos = self->contents.array.length_pos;
-    GIArgument* length_arg = &(state->out_cvalues[length_pos]);
+    GIArgument* length_arg = &state->out_cvalue(length_pos);
     GITypeTag length_tag = self->contents.array.length_tag;
     size_t length = gjs_g_argument_get_array_length(length_tag, length_arg);
 
@@ -850,8 +849,7 @@ static bool gjs_marshal_generic_inout_release(JSContext* cx,
     // the temporary C value we allocated, clearly we're responsible for
     // freeing it.
 
-    GIArgument* original_out_arg =
-        &(state->inout_original_cvalues[self->arg_pos]);
+    GIArgument* original_out_arg = &state->inout_original_cvalue(self->arg_pos);
     if (!gjs_g_argument_release_in_arg(cx, GI_TRANSFER_NOTHING,
                                        &self->type_info, original_out_arg))
         return false;
@@ -864,7 +862,7 @@ static bool gjs_marshal_explicit_array_out_release(
     JSContext* cx, GjsArgumentCache* self, GjsFunctionCallState* state,
     GIArgument* in_arg [[maybe_unused]], GIArgument* out_arg) {
     uint8_t length_pos = self->contents.array.length_pos;
-    GIArgument* length_arg = &(state->out_cvalues[length_pos]);
+    GIArgument* length_arg = &state->out_cvalue(length_pos);
     GITypeTag length_tag = self->contents.array.length_tag;
     size_t length = gjs_g_argument_get_array_length(length_tag, length_arg);
 
@@ -877,7 +875,7 @@ static bool gjs_marshal_explicit_array_in_release(
     JSContext* cx, GjsArgumentCache* self, GjsFunctionCallState* state,
     GIArgument* in_arg, GIArgument* out_arg [[maybe_unused]]) {
     uint8_t length_pos = self->contents.array.length_pos;
-    GIArgument* length_arg = &(state->in_cvalues[length_pos]);
+    GIArgument* length_arg = &state->in_cvalue(length_pos);
     GITypeTag length_tag = self->contents.array.length_tag;
     size_t length = gjs_g_argument_get_array_length(length_tag, length_arg);
 
@@ -893,7 +891,7 @@ static bool gjs_marshal_explicit_array_inout_release(
     JSContext* cx, GjsArgumentCache* self, GjsFunctionCallState* state,
     GIArgument* in_arg [[maybe_unused]], GIArgument* out_arg) {
     uint8_t length_pos = self->contents.array.length_pos;
-    GIArgument* length_arg = &(state->in_cvalues[length_pos]);
+    GIArgument* length_arg = &state->in_cvalue(length_pos);
     GITypeTag length_tag = self->contents.array.length_tag;
     size_t length = gjs_g_argument_get_array_length(length_tag, length_arg);
 
@@ -901,8 +899,7 @@ static bool gjs_marshal_explicit_array_inout_release(
     // the temporary C value we allocated, clearly we're responsible for
     // freeing it.
 
-    GIArgument* original_out_arg =
-        &(state->inout_original_cvalues[self->arg_pos]);
+    GIArgument* original_out_arg = &state->inout_original_cvalue(self->arg_pos);
     if (gjs_arg_get<void*>(original_out_arg) != gjs_arg_get<void*>(out_arg) &&
         !gjs_g_argument_release_in_array(cx, GI_TRANSFER_NOTHING,
                                          &self->type_info, length,
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 97648577..6422129d 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -21,7 +21,7 @@
 #include "gjs/enum-utils.h"
 #include "gjs/macros.h"
 
-struct GjsFunctionCallState;
+class GjsFunctionCallState;
 struct GjsArgumentCache;
 
 struct GjsArgumentMarshallers {
diff --git a/gi/function.cpp b/gi/function.cpp
index c8d336de..be84cb8a 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -840,15 +840,15 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
     }
 
     // These arrays hold argument pointers.
-    // - state.in_cvalues: C values which are passed on input (in or inout)
-    // - state.out_cvalues: C values which are returned as arguments (out or
+    // - state.in_cvalue(): C values which are passed on input (in or inout)
+    // - state.out_cvalue(): C values which are returned as arguments (out or
     //   inout)
-    // - state.inout_original_cvalues: For the special case of (inout) args, we
-    //   need to keep track of the original values we passed into the function,
-    //   in case we need to free it.
+    // - state.inout_original_cvalue(): For the special case of (inout) args,
+    //   we need to keep track of the original values we passed into the
+    //   function, in case we need to free it.
     // - ffi_arg_pointers: For passing data to FFI, we need to create another
     //   layer of indirection; this array is a pointer to an element in
-    //   state.in_cvalues or state.out_cvalues.
+    //   state.in_cvalue() or state.out_cvalue().
     // - return_value: The actual return value of the C function, i.e. not an
     //   (out) param
     //
@@ -874,7 +874,7 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
 
     if (state.is_method) {
         GjsArgumentCache* cache = &m_arguments[-state.first_arg_offset()];
-        GIArgument* in_value = &state.in_cvalues[-state.first_arg_offset()];
+        GIArgument* in_value = &state.in_cvalue(-state.first_arg_offset());
         JS::RootedValue in_js_value(context, JS::ObjectValue(*obj));
 
         if (!cache->marshallers->in(context, cache, &state, in_value,
@@ -904,7 +904,7 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
     for (gi_arg_pos = 0; gi_arg_pos < state.gi_argc;
          gi_arg_pos++, ffi_arg_pos++) {
         GjsArgumentCache* cache = &m_arguments[gi_arg_pos];
-        GIArgument* in_value = &state.in_cvalues[gi_arg_pos];
+        GIArgument* in_value = &state.in_cvalue(gi_arg_pos);
 
         gjs_debug_marshal(GJS_DEBUG_GFUNCTION,
                           "Marshalling argument '%s' in, %d/%d GI args, %u/%u "
@@ -973,7 +973,7 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
 
     if (!m_arguments[-1].skip_out()) {
         gi_type_info_extract_ffi_return_value(
-            &m_arguments[-1].type_info, &return_value, &state.out_cvalues[-1]);
+            &m_arguments[-1].type_info, &return_value, &state.out_cvalue(-1));
     }
 
     // Process out arguments and return values. This loop is skipped if we fail
@@ -981,7 +981,7 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
     js_arg_pos = 0;
     for (gi_arg_pos = -1; gi_arg_pos < state.gi_argc; gi_arg_pos++) {
         GjsArgumentCache* cache = &m_arguments[gi_arg_pos];
-        GIArgument* out_value = &state.out_cvalues[gi_arg_pos];
+        GIArgument* out_value = &state.out_cvalue(gi_arg_pos);
 
         gjs_debug_marshal(GJS_DEBUG_GFUNCTION,
                           "Marshalling argument '%s' out, %d/%d GI args",
@@ -1033,8 +1033,8 @@ bool Function::finish_invoke(JSContext* cx, const JS::CallArgs& args,
          gi_arg_pos < state->gi_argc && ffi_arg_pos < ffi_arg_max;
          gi_arg_pos++, ffi_arg_pos++) {
         GjsArgumentCache* cache = &m_arguments[gi_arg_pos];
-        GIArgument* in_value = &state->in_cvalues[gi_arg_pos];
-        GIArgument* out_value = &state->out_cvalues[gi_arg_pos];
+        GIArgument* in_value = &state->in_cvalue(gi_arg_pos);
+        GIArgument* out_value = &state->out_cvalue(gi_arg_pos);
 
         gjs_debug_marshal(
             GJS_DEBUG_GFUNCTION,
diff --git a/gi/function.h b/gi/function.h
index 759a01dd..d01e9aa8 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -85,10 +85,12 @@ using GjsAutoCallbackTrampoline =
                    gjs_callback_trampoline_unref, gjs_callback_trampoline_ref>;
 
 // Stack allocation only!
-struct GjsFunctionCallState {
-    GIArgument* in_cvalues;
-    GIArgument* out_cvalues;
-    GIArgument* inout_original_cvalues;
+class GjsFunctionCallState {
+    GIArgument* m_in_cvalues;
+    GIArgument* m_out_cvalues;
+    GIArgument* m_inout_original_cvalues;
+
+ public:
     std::unordered_set<GIArgument*> ignore_release;
     JS::RootedObject instance_object;
     JS::RootedValueVector return_values;
@@ -107,15 +109,15 @@ struct GjsFunctionCallState {
           can_throw_gerror(g_callable_info_can_throw_gerror(callable)),
           is_method(g_callable_info_is_method(callable)) {
         int size = gi_argc + first_arg_offset();
-        in_cvalues = new GIArgument[size] + first_arg_offset();
-        out_cvalues = new GIArgument[size] + first_arg_offset();
-        inout_original_cvalues = new GIArgument[size] + first_arg_offset();
+        m_in_cvalues = new GIArgument[size];
+        m_out_cvalues = new GIArgument[size];
+        m_inout_original_cvalues = new GIArgument[size];
     }
 
     ~GjsFunctionCallState() {
-        delete[](in_cvalues - first_arg_offset());
-        delete[](out_cvalues - first_arg_offset());
-        delete[](inout_original_cvalues - first_arg_offset());
+        delete[] m_in_cvalues;
+        delete[] m_out_cvalues;
+        delete[] m_inout_original_cvalues;
     }
 
     GjsFunctionCallState(const GjsFunctionCallState&) = delete;
@@ -123,6 +125,18 @@ struct GjsFunctionCallState {
 
     constexpr int first_arg_offset() const { return is_method ? 2 : 1; }
 
+    constexpr GIArgument& in_cvalue(int index) const {
+        return m_in_cvalues[index + first_arg_offset()];
+    }
+
+    constexpr GIArgument& out_cvalue(int index) const {
+        return m_out_cvalues[index + first_arg_offset()];
+    }
+
+    constexpr GIArgument& inout_original_cvalue(int index) const {
+        return m_inout_original_cvalues[index + first_arg_offset()];
+    }
+
     constexpr bool did_throw_gerror() const {
         return can_throw_gerror && local_error;
     }