gobject: limit required stack by heap allocating parameter list in g_object_new_valist()th/gobject-new-parameter-list

The previous code consumed a larger additional amount of stack space.
That is because it would allocate the temporary buffer for GValues on
the stack with "g_newa (GValue, 1)" and thus the required stack
space grew with the number of arguments. Granted, this is already
a variadic C function, so the caller already placed that many elements
on the stack. For example, on the stack there are the property names
and the pointers to the arguments, which should amount to roughly
O(n_args * 16) (on 64 bit, with pointers being 8 bytes large).

That is not bad, because it means in the previous version the stack space
would grow linear with the already used stack space. However, a GValue is
an additional 24 bytes (on 64 bit), which probably more than doubles the
required stack space. Let's avoid that, by allocating the temporary list
on the heap after a certain threshold. This probably more than doubles the
number of possible arguments before the stack overflows.

Also, previously the heap allocated "params" array only grew one element
per iteration. Of course, it is likely that libc anyway reallocates
the buffers by growing the space exponentially. So realloc(ptr, 1)
probably does not O() scale worse than doubling the buffer sizes ourselves.
However, it seems clearer to keep track of the allocated sizes ourself, and
only call realloc() when we determine that we are out of space.
Especially because we need to update the value pointers on reallocation.

Note that we now require a heap allocation both for the "params" and the
"values" list. Theoretically that could be combined by using one buffer
for both. But that would make the code more complicated.

Now we pre-allocate buffers for 16 elements on the stack. That
is (16 * (16 + 24) bytes (or 640 bytes) on the stack. I think that
is still acceptable.

1 files changed, 35 insertions, 15 deletions

diff --git a/gobject/gobject.c b/gobject/gobject.c
index 3e860da26..32d554874 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -2218,13 +2218,15 @@ g_object_new_valist (GType        object_type,
 
   if (first_property_name)
     {
-      GObjectConstructParam stack_params[16];
-      GObjectConstructParam *params;
+      GObjectConstructParam params_stack[16];
+      GValue values_stack[G_N_ELEMENTS (params_stack)];
       const gchar *name;
-      gint n_params = 0;
+      GObjectConstructParam *params = params_stack;
+      GValue *values = values_stack;
+      guint n_params = 0;
+      guint n_params_alloc = G_N_ELEMENTS (params_stack);
 
       name = first_property_name;
-      params = stack_params;
 
       do
         {
@@ -2236,24 +2238,39 @@ g_object_new_valist (GType        object_type,
           if (!g_object_new_is_valid_property (object_type, pspec, name, params, n_params))
             break;
 
-          if (n_params == 16)
+          if (G_UNLIKELY (n_params == n_params_alloc))
             {
-              params = g_new (GObjectConstructParam, n_params + 1);
-              memcpy (params, stack_params, sizeof stack_params);
+              guint i;
+
+              if (n_params_alloc == G_N_ELEMENTS (params_stack))
+                {
+                  n_params_alloc = G_N_ELEMENTS (params_stack) * 2u;
+                  params = g_new (GObjectConstructParam, n_params_alloc);
+                  values = g_new (GValue, n_params_alloc);
+                  memcpy (params, params_stack, sizeof (GObjectConstructParam) * n_params);
+                  memcpy (values, values_stack, sizeof (GValue) * n_params);
+                }
+              else
+                {
+                  n_params_alloc *= 2u;
+                  params = g_realloc (params, sizeof (GObjectConstructParam) * n_params_alloc);
+                  values = g_realloc (values, sizeof (GValue) * n_params_alloc);
+                }
+
+              for (i = 0; i < n_params; i++)
+                params[i].value = &values[i];
             }
-          else if (n_params > 16)
-            params = g_renew (GObjectConstructParam, params, n_params + 1);
 
           params[n_params].pspec = pspec;
-          params[n_params].value = g_newa (GValue, 1);
-          memset (params[n_params].value, 0, sizeof (GValue));
+          params[n_params].value = &values[n_params];
+          memset (&values[n_params], 0, sizeof (GValue));
 
-          G_VALUE_COLLECT_INIT (params[n_params].value, pspec->value_type, var_args, 0, &error);
+          G_VALUE_COLLECT_INIT (&values[n_params], pspec->value_type, var_args, 0, &error);
 
           if (error)
             {
               g_critical ("%s: %s", G_STRFUNC, error);
-              g_value_unset (params[n_params].value);
+              g_value_unset (&values[n_params]);
               g_free (error);
               break;
             }
@@ -2267,8 +2284,11 @@ g_object_new_valist (GType        object_type,
       while (n_params--)
         g_value_unset (params[n_params].value);
 
-      if (params != stack_params)
-        g_free (params);
+      if (G_UNLIKELY (n_params_alloc != G_N_ELEMENTS (params_stack)))
+        {
+          g_free (params);
+          g_free (values);
+        }
     }
   else
     /* Fast case: no properties passed in. */