shared: avoid allocating temporary buffer for nm_utils_named_values_from_strdict()

Iterating hash tables gives an undefined order. Often we want to have
a stable order, for example when printing the content of a hash or
when converting it to a "a{sv}" variant.

How to achieve that best? I think we should only iterate the hash once,
and not require additional lookups. nm_utils_named_values_from_strdict()
achieves that by returning the key and the value together. Also, often
we only need the list for a short time, so we can avoid heap allocating
the list, if it is short enough. This works by allowing the caller to
provide a pre-allocated buffer (usually on the stack) and only as fallback
allocate a new list.

4 files changed, 51 insertions, 25 deletions

diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index 8986e434e5..da759efcaa 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -368,7 +368,7 @@ static void
 _ensure_options_idx_cache (NMSettingBondPrivate *priv)
 {
 	if (!G_UNLIKELY (priv->options_idx_cache))
-		priv->options_idx_cache = nm_utils_named_values_from_str_dict_with_sort (priv->options, NULL, _get_option_sort, NULL);
+		priv->options_idx_cache = nm_utils_named_values_from_strdict_full (priv->options, NULL, _get_option_sort, NULL, NULL, 0, NULL);
 }
 
 /**
diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index 39e98a31df..baa91646e8 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -2909,28 +2909,44 @@ nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll)
 
 /*****************************************************************************/
 
+G_STATIC_ASSERT (G_STRUCT_OFFSET (NMUtilsNamedValue, name) == 0);
+
 NMUtilsNamedValue *
-nm_utils_named_values_from_str_dict_with_sort (GHashTable *hash,
-                                               guint *out_len,
-                                               GCompareDataFunc compare_func,
-                                               gpointer user_data)
+nm_utils_named_values_from_strdict_full (GHashTable *hash,
+                                         guint *out_len,
+                                         GCompareDataFunc compare_func,
+                                         gpointer user_data,
+                                         NMUtilsNamedValue *provided_buffer,
+                                         guint provided_buffer_len,
+                                         NMUtilsNamedValue **out_allocated_buffer)
 {
 	GHashTableIter iter;
 	NMUtilsNamedValue *values;
 	guint i, len;
 
+	nm_assert (provided_buffer_len == 0 || provided_buffer);
+	nm_assert (!out_allocated_buffer || !*out_allocated_buffer);
+
 	if (   !hash
 	    || !(len = g_hash_table_size (hash))) {
 		NM_SET_OUT (out_len, 0);
 		return NULL;
 	}
 
+	if (provided_buffer_len >= len + 1) {
+		/* the buffer provided by the caller is large enough. Use it. */
+		values = provided_buffer;
+	} else {
+		/* allocate a new buffer. */
+		values = g_new (NMUtilsNamedValue, len + 1);
+		NM_SET_OUT (out_allocated_buffer, values);
+	}
+
 	i = 0;
-	values = g_new (NMUtilsNamedValue, len + 1);
 	g_hash_table_iter_init (&iter, hash);
 	while (g_hash_table_iter_next (&iter,
 	                               (gpointer *) &values[i].name,
-	                               (gpointer *) &values[i].value_ptr))
+	                               &values[i].value_ptr))
 		i++;
 	nm_assert (i == len);
 	values[i].name = NULL;
@@ -4925,7 +4941,7 @@ _nm_utils_format_variant_attributes_full (GString *str,
 
 	for (i = 0; i < num_values; i++) {
 		name = values[i].name;
-		variant = (GVariant *) values[i].value_ptr;
+		variant = values[i].value_ptr;
 		value = NULL;
 
 		if (g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32))
@@ -4969,17 +4985,24 @@ _nm_utils_format_variant_attributes (GHashTable *attributes,
                                      char attr_separator,
                                      char key_value_separator)
 {
+	gs_free NMUtilsNamedValue *values_free = NULL;
+	NMUtilsNamedValue values_prepared[20];
+	const NMUtilsNamedValue *values;
 	GString *str = NULL;
-	gs_free NMUtilsNamedValue *values = NULL;
 	guint len;
 
 	g_return_val_if_fail (attr_separator, NULL);
 	g_return_val_if_fail (key_value_separator, NULL);
 
-	if (!attributes || !g_hash_table_size (attributes))
+	if (!attributes)
 		return NULL;
 
-	values = nm_utils_named_values_from_str_dict (attributes, &len);
+	values = nm_utils_named_values_from_strdict (attributes,
+	                                             &len,
+	                                             values_prepared,
+	                                             &values_free);
+	if (len == 0)
+		return NULL;
 
 	str = g_string_new ("");
 	_nm_utils_format_variant_attributes_full (str,
diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h
index 7d4611bff6..a74422043a 100644
--- a/shared/nm-glib-aux/nm-shared-utils.h
+++ b/shared/nm-glib-aux/nm-shared-utils.h
@@ -1452,18 +1452,16 @@ typedef struct {
 
 #define NM_UTILS_NAMED_VALUE_INIT(n, v)     { .name = (n), .value_ptr = (v) }
 
-NMUtilsNamedValue *nm_utils_named_values_from_str_dict_with_sort (GHashTable *hash,
-                                                                  guint *out_len,
-                                                                  GCompareDataFunc compare_func,
-                                                                  gpointer user_data);
-
-static inline NMUtilsNamedValue *
-nm_utils_named_values_from_str_dict (GHashTable *hash, guint *out_len)
-{
-	G_STATIC_ASSERT (G_STRUCT_OFFSET (NMUtilsNamedValue, name) == 0);
-
-	return nm_utils_named_values_from_str_dict_with_sort (hash, out_len, nm_strcmp_p_with_data, NULL);
-}
+NMUtilsNamedValue *nm_utils_named_values_from_strdict_full (GHashTable *hash,
+                                                            guint *out_len,
+                                                            GCompareDataFunc compare_func,
+                                                            gpointer user_data,
+                                                            NMUtilsNamedValue *provided_buffer,
+                                                            guint provided_buffer_len,
+                                                            NMUtilsNamedValue **out_allocated_buffer);
+
+#define nm_utils_named_values_from_strdict(hash, out_len, array, out_allocated_buffer) \
+	nm_utils_named_values_from_strdict_full ((hash), (out_len), nm_strcmp_p_with_data, NULL, (array), G_N_ELEMENTS (array), (out_allocated_buffer))
 
 gssize nm_utils_named_value_list_find (const NMUtilsNamedValue *arr,
                                        gsize len,
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 019d1e600f..2492fc10aa 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -4111,12 +4111,17 @@ nm_utils_parse_dns_domain (const char *domain, gboolean *is_routing)
 GVariant *
 nm_utils_strdict_to_variant (GHashTable *options)
 {
+	gs_free NMUtilsNamedValue *values_free = NULL;
+	NMUtilsNamedValue values_prepared[20];
+	const NMUtilsNamedValue *values;
 	GVariantBuilder builder;
-	gs_free NMUtilsNamedValue *values = NULL;
 	guint i;
 	guint n;
 
-	values = nm_utils_named_values_from_str_dict (options, &n);
+	values = nm_utils_named_values_from_strdict (options,
+	                                             &n,
+	                                             values_prepared,
+	                                             &values_free);
 
 	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
 	for (i = 0; i < n; i++) {