libnm: avoid unnecessary copies accessing NMIPRoute's attributes

We want to support large number of routes. Reduce the number
of copies, by adding internal accessor functions.

Also, work around a complaint from coverity:

  46. NetworkManager-1.9.2/libnm-core/nm-utils.c:1987:
  dereference: Dereferencing a null pointer "names".
This commit is contained in:
Thomas Haller 2017-10-30 12:23:34 +01:00
parent 4a8a5495a9
commit f3146de41b
6 changed files with 89 additions and 40 deletions

View file

@ -42,7 +42,7 @@
static char **
_ip_config_get_routes (NMIPConfig *cfg)
{
gs_unref_hashtable GHashTable *hash = g_hash_table_new (nm_str_hash, g_str_equal);
gs_unref_hashtable GHashTable *hash = NULL;
GPtrArray *ptr_array;
char **arr;
guint i;
@ -55,10 +55,10 @@ _ip_config_get_routes (NMIPConfig *cfg)
for (i = 0; i < ptr_array->len; i++) {
NMIPRoute *route = g_ptr_array_index (ptr_array, i);
gs_strfreev char **names = NULL;
gs_free char *attributes = NULL;
gsize j;
GString *str;
guint64 metric;
gs_free char *attributes = NULL;
str = g_string_new (NULL);
g_string_append_printf (str,
@ -76,13 +76,20 @@ _ip_config_get_routes (NMIPConfig *cfg)
}
names = nm_ip_route_get_attribute_names (route);
g_hash_table_remove_all (hash);
for (j = 0; names && names[j]; j++)
g_hash_table_insert (hash, names[j], nm_ip_route_get_attribute (route, names[j]));
attributes = nm_utils_format_variant_attributes (hash, ',', '=');
if (attributes) {
g_string_append (str, ", ");
g_string_append (str, attributes);
if (names[0]) {
if (!hash)
hash = g_hash_table_new (nm_str_hash, g_str_equal);
else
g_hash_table_remove_all (hash);
for (j = 0; names[j]; j++)
g_hash_table_insert (hash, names[j], nm_ip_route_get_attribute (route, names[j]));
attributes = nm_utils_format_variant_attributes (hash, ',', '=');
if (attributes) {
g_string_append (str, ", ");
g_string_append (str, attributes);
}
}
arr[i] = g_string_free (str, FALSE);

View file

@ -199,6 +199,8 @@ GHashTable *_nm_utils_copy_strdict (GHashTable *strdict);
typedef gpointer (*NMUtilsCopyFunc) (gpointer);
gboolean _nm_ip_route_attribute_validate_all (const NMIPRoute *route);
const char **_nm_ip_route_get_attribute_names (const NMIPRoute *route, gboolean sorted, guint *out_length);
GHashTable *_nm_ip_route_get_attributes_direct (NMIPRoute *route);
static inline void
_nm_auto_ip_route_unref (NMIPRoute **v)

View file

@ -193,14 +193,9 @@ write_ip_values (GKeyFile *file,
if (is_route) {
gs_free char *attributes = NULL;
gs_strfreev char **names = NULL;
gs_unref_hashtable GHashTable *hash = g_hash_table_new (g_str_hash, g_str_equal);
int j;
names = nm_ip_route_get_attribute_names (array->pdata[i]);
for (j = 0; names && names[j]; j++)
g_hash_table_insert (hash, names[j], nm_ip_route_get_attribute (array->pdata[i], names[j]));
GHashTable *hash;
hash = _nm_ip_route_get_attributes_direct (array->pdata[i]);
attributes = nm_utils_format_variant_attributes (hash, ',', '=');
if (attributes) {
g_strlcat (key_name, "_options", sizeof (key_name));

View file

@ -1112,6 +1112,54 @@ nm_ip_route_set_metric (NMIPRoute *route,
route->metric = metric;
}
GHashTable *
_nm_ip_route_get_attributes_direct (NMIPRoute *route)
{
nm_assert (route);
return route->attributes;
}
/**
* _nm_ip_route_get_attribute_names:
* @route: the #NMIPRoute
* @sorted: whether to sort the names. Otherwise, their order is
* undefined and unstable.
* @out_length: (allow-none): (out): the number of elements
*
* Gets an array of attribute names defined on @route.
*
* Returns: (array length=out_length) (transfer container): a %NULL-terminated array
* of attribute names or %NULL if there are no attributes. The order of the returned
* names is undefined.
**/
const char **
_nm_ip_route_get_attribute_names (const NMIPRoute *route, gboolean sorted, guint *out_length)
{
const char **names;
guint length;
g_return_val_if_fail (route != NULL, NULL);
if ( !route->attributes
|| !g_hash_table_size (route->attributes)) {
NM_SET_OUT (out_length, 0);
return NULL;
}
names = (const char **) g_hash_table_get_keys_as_array (route->attributes, &length);
if ( sorted
&& length > 1) {
g_qsort_with_data (names,
length,
sizeof (char *),
nm_strcmp_p_with_data,
NULL);
}
NM_SET_OUT (out_length, length);
return names;
}
/**
* nm_ip_route_get_attribute_names:
* @route: the #NMIPRoute
@ -1123,23 +1171,21 @@ nm_ip_route_set_metric (NMIPRoute *route,
char **
nm_ip_route_get_attribute_names (NMIPRoute *route)
{
GHashTableIter iter;
const char *key;
GPtrArray *names;
char **names;
guint i, len;
g_return_val_if_fail (route != NULL, NULL);
names = g_ptr_array_new ();
names = (char **) _nm_ip_route_get_attribute_names (route, TRUE, &len);
if (!names)
return g_new0 (char *, 1);
if (route->attributes) {
g_hash_table_iter_init (&iter, route->attributes);
while (g_hash_table_iter_next (&iter, (gpointer *) &key, NULL))
g_ptr_array_add (names, g_strdup (key));
nm_assert (len > 0 && names && names[len] == NULL);
for (i = 0; i < len; i++) {
nm_assert (names[i]);
names[i] = g_strdup (names[i]);
}
g_ptr_array_sort (names, nm_strcmp_p);
g_ptr_array_add (names, NULL);
return (char **) g_ptr_array_free (names, FALSE);
return names;
}
/**

View file

@ -1962,8 +1962,8 @@ nm_utils_ip_routes_to_variant (GPtrArray *routes)
for (i = 0; i < routes->len; i++) {
NMIPRoute *route = routes->pdata[i];
GVariantBuilder route_builder;
char **names;
int n;
gs_free const char **names = NULL;
guint j, len;
g_variant_builder_init (&route_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (&route_builder, "{sv}",
@ -1983,13 +1983,12 @@ nm_utils_ip_routes_to_variant (GPtrArray *routes)
g_variant_new_uint32 ((guint32) nm_ip_route_get_metric (route)));
}
names = nm_ip_route_get_attribute_names (route);
for (n = 0; names[n]; n++) {
names = _nm_ip_route_get_attribute_names (route, TRUE, &len);
for (j = 0; j < len; j++) {
g_variant_builder_add (&route_builder, "{sv}",
names[n],
nm_ip_route_get_attribute (route, names[n]));
names[j],
nm_ip_route_get_attribute (route, names[j]));
}
g_strfreev (names);
g_variant_builder_add (&builder, "a{sv}", &route_builder);
}

View file

@ -1867,18 +1867,18 @@ write_connection_setting (NMSettingConnection *s_con, shvarFile *ifcfg)
static char *
get_route_attributes_string (NMIPRoute *route, int family)
{
gs_strfreev char **names = NULL;
gs_free const char **names = NULL;
GVariant *attr, *lock;
GString *str;
int i;
guint i, len;
names = nm_ip_route_get_attribute_names (route);
if (!names || !names[0])
names = _nm_ip_route_get_attribute_names (route, TRUE, &len);
if (!len)
return NULL;
str = g_string_new ("");
for (i = 0; names[i]; i++) {
for (i = 0; i < len; i++) {
attr = nm_ip_route_get_attribute (route, names[i]);
if (!nm_ip_route_attribute_validate (names[i], attr, family, NULL, NULL))