From 5cd0fdb2dd6b885364ce044d7e0fa7d10a9fcf7c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Oct 2023 16:45:11 +0200 Subject: [PATCH 1/2] all: use nm_strv_contains() instead of nm_strv_find_first() for membership check nm_strv_find_first() is useful (and used) to find the first index (if any). I can thus also used to check for membership. However, we also have nm_strv_contains(), which seems better for readability, when we check for membership. Use it. --- src/core/NetworkManagerUtils.c | 2 +- src/core/devices/nm-device-bond.c | 2 +- src/core/devices/nm-device.c | 2 +- src/core/dns/nm-dns-manager.c | 8 ++++---- src/core/nm-config.c | 7 +++---- src/core/nm-l3-config-data.c | 2 +- src/core/settings/nm-settings.c | 2 +- src/core/supplicant/nm-supplicant-settings-verify.c | 2 +- src/libnm-client-impl/nm-libnm-utils.c | 2 +- src/libnm-core-impl/nm-connection.c | 2 +- src/libnm-core-impl/nm-team-utils.c | 6 +++--- src/libnm-core-impl/nm-vpn-plugin-info.c | 6 +++--- src/libnm-glib-aux/nm-shared-utils.c | 6 ++---- src/libnm-platform/nm-platform-utils.c | 2 +- src/nm-initrd-generator/nmi-cmdline-reader.c | 4 ++-- 15 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 8606082c35..7b6a20ab42 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -120,7 +120,7 @@ get_new_connection_name(NMConnection *const *existing_connections, * connection id. */ temp = g_strdup_printf(C_("connection id fallback", "%s %u"), fallback_prefix, i); - if (nm_strv_find_first(existing_names, existing_len, temp) < 0) + if (!nm_strv_contains(existing_names, existing_len, temp)) return temp; g_free(temp); diff --git a/src/core/devices/nm-device-bond.c b/src/core/devices/nm-device-bond.c index bc5d3bfcae..85d625c422 100644 --- a/src/core/devices/nm-device-bond.c +++ b/src/core/devices/nm-device-bond.c @@ -292,7 +292,7 @@ set_arp_targets(NMDevice *device, const char *cur_arp_ip_target, const char *new } } - if (nm_strv_find_first(new_strv, i, s) < 0) + if (!nm_strv_contains(new_strv, i, s)) new_strv[j++] = s; } new_strv[j] = NULL; diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 9af77ece3c..71b597fb34 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -13138,7 +13138,7 @@ _nm_device_hash_check_invalid_keys(GHashTable *hash, g_hash_table_iter_init(&iter, hash); while (g_hash_table_iter_next(&iter, (gpointer *) &k, NULL)) { - if (nm_strv_find_first(whitelist, -1, k) < 0) { + if (!nm_strv_contains(whitelist, -1, k)) { first_invalid_key = k; break; } diff --git a/src/core/dns/nm-dns-manager.c b/src/core/dns/nm-dns-manager.c index 231ef72f49..f8f12fbcbc 100644 --- a/src/core/dns/nm-dns-manager.c +++ b/src/core/dns/nm-dns-manager.c @@ -1876,8 +1876,8 @@ plugin_skip:; nameservers = g_new0(char *, 2); nameservers[0] = g_strdup(lladdr); - need_edns0 = nm_strv_find_first(options, -1, NM_SETTING_DNS_OPTION_EDNS0) < 0; - need_trust = nm_strv_find_first(options, -1, NM_SETTING_DNS_OPTION_TRUST_AD) < 0; + need_edns0 = !nm_strv_contains(options, -1, NM_SETTING_DNS_OPTION_EDNS0); + need_trust = !nm_strv_contains(options, -1, NM_SETTING_DNS_OPTION_TRUST_AD); if (need_edns0 || need_trust) { gsize len; @@ -2401,7 +2401,7 @@ _resolvconf_resolved_managed(void) * We want to handle that, because systemd-resolved might not * have started yet. */ full_path = g_file_read_link(_PATH_RESCONF, NULL); - if (nm_strv_find_first(RESOLVED_PATHS, G_N_ELEMENTS(RESOLVED_PATHS), full_path) >= 0) + if (nm_strv_contains(RESOLVED_PATHS, G_N_ELEMENTS(RESOLVED_PATHS), full_path)) return TRUE; /* see if resolv.conf is a symlink that resolves exactly one @@ -2413,7 +2413,7 @@ _resolvconf_resolved_managed(void) * We want to handle that, because systemd-resolved might not * have started yet. */ real_path = realpath(_PATH_RESCONF, NULL); - if (nm_strv_find_first(RESOLVED_PATHS, G_N_ELEMENTS(RESOLVED_PATHS), real_path) >= 0) + if (nm_strv_contains(RESOLVED_PATHS, G_N_ELEMENTS(RESOLVED_PATHS), real_path)) return TRUE; /* fall-through and resolve the symlink, to check the file diff --git a/src/core/nm-config.c b/src/core/nm-config.c index b8df41b723..31d758f882 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -1129,12 +1129,12 @@ read_config(GKeyFile *keyfile, /* merge the string lists, by omitting duplicates. */ for (iter_val = old_val; iter_val && *iter_val; iter_val++) { - if (last_char != '-' || nm_strv_find_first(new_val, -1, *iter_val) < 0) + if (last_char != '-' || !nm_strv_contains(new_val, -1, *iter_val)) g_ptr_array_add(new, g_strdup(*iter_val)); } for (iter_val = new_val; iter_val && *iter_val; iter_val++) { /* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */ - if (last_char == '+' && nm_strv_find_first(old_val, -1, *iter_val) < 0) + if (last_char == '+' && !nm_strv_contains(old_val, -1, *iter_val)) g_ptr_array_add(new, *iter_val); else g_free(*iter_val); @@ -3019,8 +3019,7 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps gboolean nm_config_kernel_command_line_nm_debug(void) { - return (nm_strv_find_first(nm_utils_proc_cmdline_split(), -1, NM_CONFIG_KERNEL_CMDLINE_NM_DEBUG) - >= 0); + return nm_strv_contains(nm_utils_proc_cmdline_split(), -1, NM_CONFIG_KERNEL_CMDLINE_NM_DEBUG); } /*****************************************************************************/ diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 96274ba997..f7f81d93a7 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -324,7 +324,7 @@ _strv_ptrarray_merge(GPtrArray **p_dst, const GPtrArray *src) const char *s = src->pdata[i]; if (dst_initial_len > 0 - && nm_strv_find_first((const char *const *) ((*p_dst)->pdata), dst_initial_len, s) >= 0) + && nm_strv_contains((const char *const *) ((*p_dst)->pdata), dst_initial_len, s)) continue; g_ptr_array_add(*p_dst, g_strdup(s)); diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index 8796de3612..7fc2029883 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -3530,7 +3530,7 @@ load_plugins(NMSettings *self, const char *const *plugins, GError **error) continue; } - if (nm_strv_find_first(plugins, iter - plugins, pname) >= 0) { + if (nm_strv_contains(plugins, iter - plugins, pname)) { /* the plugin is already mentioned in the list previously. * Don't load a duplicate. */ continue; diff --git a/src/core/supplicant/nm-supplicant-settings-verify.c b/src/core/supplicant/nm-supplicant-settings-verify.c index fc40badecd..8f2561a6a9 100644 --- a/src/core/supplicant/nm-supplicant-settings-verify.c +++ b/src/core/supplicant/nm-supplicant-settings-verify.c @@ -228,7 +228,7 @@ validate_type_keyword(const struct Opt *opt, const char *value, const guint32 le s++; } - if (nm_strv_find_first(opt->str_allowed, -1, value) < 0) + if (!nm_strv_contains(opt->str_allowed, -1, value)) return FALSE; if (!s) diff --git a/src/libnm-client-impl/nm-libnm-utils.c b/src/libnm-client-impl/nm-libnm-utils.c index 398c6ebcfa..dfd832890a 100644 --- a/src/libnm-client-impl/nm-libnm-utils.c +++ b/src/libnm-client-impl/nm-libnm-utils.c @@ -299,7 +299,7 @@ _fixup_string(const char *desc, if (eow) *eow = '\0'; - if (nm_strv_find_first(ignored_words, -1, p) >= 0) + if (nm_strv_contains(ignored_words, -1, p)) goto next; l = strlen(p); diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 6ffc750ac9..24304f7d62 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -986,7 +986,7 @@ _normalize_connection_secondaries(NMConnection *self) if (!nm_uuid_is_valid_nm(s, &uuid_is_normalized, uuid_normalized)) continue; - if (nm_strv_find_first(strv, j, uuid_is_normalized ? uuid_normalized : s) >= 0) + if (nm_strv_contains(strv, j, uuid_is_normalized ? uuid_normalized : s)) continue; strv[j++] = uuid_is_normalized ? g_strdup(uuid_normalized) : g_steal_pointer(&s); diff --git a/src/libnm-core-impl/nm-team-utils.c b/src/libnm-core-impl/nm-team-utils.c index 2f62f6c613..1aae11e49d 100644 --- a/src/libnm-core-impl/nm-team-utils.c +++ b/src/libnm-core-impl/nm-team-utils.c @@ -2244,7 +2244,7 @@ _team_setting_verify_properties(const NMTeamSetting *self, GError **error) } else if (attr_data->value_type == NM_VALUE_TYPE_STRING) { const char *v = *((const char *const *) p_field); - if (nm_strv_find_first(attr_data->range.r_string.valid_names, -1, v) < 0) { + if (!nm_strv_contains(attr_data->range.r_string.valid_names, -1, v)) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING, @@ -2263,7 +2263,7 @@ _team_setting_verify_properties(const NMTeamSetting *self, GError **error) for (i = 0; i < self->d.master.runner_tx_hash->len; i++) { const char *val = self->d.master.runner_tx_hash->pdata[i]; - if (!val || (nm_strv_find_first(_valid_names_runner_tx_hash, -1, val) < 0)) { + if (!val || !nm_strv_contains(_valid_names_runner_tx_hash, -1, val)) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING, @@ -2289,7 +2289,7 @@ _team_setting_verify_properties(const NMTeamSetting *self, GError **error) if (!_team_setting_has_field(self, attr_data)) continue; if (self->d.master.runner - && (nm_strv_find_first(e->valid_runners, -1, self->d.master.runner) >= 0)) + && nm_strv_contains(e->valid_runners, -1, self->d.master.runner)) continue; if (e->valid_runners[1] == NULL) { g_set_error(error, diff --git a/src/libnm-core-impl/nm-vpn-plugin-info.c b/src/libnm-core-impl/nm-vpn-plugin-info.c index d2ce9ed827..aaeefe5ed1 100644 --- a/src/libnm-core-impl/nm-vpn-plugin-info.c +++ b/src/libnm-core-impl/nm-vpn-plugin-info.c @@ -337,7 +337,7 @@ nm_vpn_plugin_info_list_load(void) uid = getuid(); for (i = 0; i < G_N_ELEMENTS(dir); i++) { - if (!dir[i] || nm_strv_find_first(dir, i, dir[i]) >= 0) + if (!dir[i] || nm_strv_contains(dir, i, dir[i])) continue; infos = _nm_vpn_plugin_info_list_load_dir(dir[i], TRUE, uid, NULL, NULL); @@ -552,7 +552,7 @@ _list_find_by_service(GSList *list, const char *name, const char *service) if (name && !nm_streq(name, priv->name)) continue; if (service && !nm_streq(priv->service, service) - && (nm_strv_find_first(priv->aliases, -1, service) < 0)) + && !nm_strv_contains(priv->aliases, -1, service)) continue; return list->data; @@ -639,7 +639,7 @@ nm_vpn_plugin_info_list_find_service_type(GSList *list, const char *name) /* check the hard-coded list of short-names. They all have the same * well-known prefix org.freedesktop.NetworkManager and the name. */ - if (nm_strv_find_first(known_names, G_N_ELEMENTS(known_names), name) >= 0) + if (nm_strv_contains(known_names, G_N_ELEMENTS(known_names), name)) return g_strdup_printf("%s.%s", NM_DBUS_INTERFACE, name); /* try, if there exists a plugin with @name under org.freedesktop.NetworkManager. diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index d391261f05..05b6750323 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -2089,8 +2089,7 @@ nm_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repe j = 0; for (i = 0; strv[i]; i++) { - if ((skip_empty && !*strv[i]) - || (skip_repeated && nm_strv_find_first(strv, j, strv[i]) >= 0)) + if ((skip_empty && !*strv[i]) || (skip_repeated && nm_strv_contains(strv, j, strv[i]))) continue; strv[j++] = strv[i]; } @@ -2117,8 +2116,7 @@ nm_strv_cleanup(char **strv, gboolean strip_whitespace, gboolean skip_empty, gbo return strv; j = 0; for (i = 0; strv[i]; i++) { - if ((skip_empty && !*strv[i]) - || (skip_repeated && nm_strv_find_first(strv, j, strv[i]) >= 0)) + if ((skip_empty && !*strv[i]) || (skip_repeated && nm_strv_contains(strv, j, strv[i]))) g_free(strv[i]); else strv[j++] = strv[i]; diff --git a/src/libnm-platform/nm-platform-utils.c b/src/libnm-platform/nm-platform-utils.c index 32c4732d3d..bba822a7b3 100644 --- a/src/libnm-platform/nm-platform-utils.c +++ b/src/libnm-platform/nm-platform-utils.c @@ -554,7 +554,7 @@ _ASSERT_ethtool_feature_infos(void) for (k = 0; k < inf->n_kernel_names; k++) { const char *name = inf->kernel_names[k]; - g_assert(nm_strv_find_first(inf->kernel_names, k, name) < 0); + g_assert(!nm_strv_contains(inf->kernel_names, k, name)); /* these offload features are only informational and cannot be set from user-space * (NETIF_F_NEVER_CHANGE). We should not track them in _ethtool_feature_infos. */ diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c index 89abed7d5d..2601f07b03 100644 --- a/src/nm-initrd-generator/nmi-cmdline-reader.c +++ b/src/nm-initrd-generator/nmi-cmdline-reader.c @@ -486,12 +486,12 @@ _parse_ip_method(const char *kind) nm_strv_sort(strv, -1); nm_strv_cleanup_const(strv, TRUE, TRUE); - if (nm_strv_find_first(strv, -1, "auto") >= 0) { + if (nm_strv_contains(strv, -1, "auto")) { /* if "auto" is present, then "dhcp4", "dhcp6", and "local6" is implied. */ _strv_remove(strv, "dhcp4"); _strv_remove(strv, "dhcp6"); _strv_remove(strv, "local6"); - } else if (nm_strv_find_first(strv, -1, "dhcp6") >= 0) { + } else if (nm_strv_contains(strv, -1, "dhcp6")) { /* if "dhcp6" is present, then "local6" is implied. */ _strv_remove(strv, "local6"); } From 3cb10bdd1e9fba6a8a8148a0583b1332303aeeac Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Oct 2023 17:11:04 +0200 Subject: [PATCH 2/2] glib-aux/trivial: rename arguments in nm_strv_cleanup() function "skip_repeated" sounds as if the function would only drop duplicate elements that follow each other (in which case, the operation would be O(n)). But it does search the entire array to prevent duplicates (resulting in O(n^2)). Rename the argument "skip_repeated" to "no_duplicates" to make that clearer. Also, rename "skip_{empty,duplicates}" to "no_{empty,duplicates}". The function removes those elements from the list, so "skip" is a bit misleading too. --- src/libnm-glib-aux/nm-shared-utils.c | 12 ++++++------ src/libnm-glib-aux/nm-shared-utils.h | 8 +++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 05b6750323..7e04b690a3 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -2076,7 +2076,7 @@ nm_strv_is_same_unordered(const char *const *strv1, } const char ** -nm_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repeated) +nm_strv_cleanup_const(const char **strv, gboolean no_empty, gboolean no_duplicates) { gsize i; gsize j; @@ -2084,12 +2084,12 @@ nm_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repe if (!strv || !*strv) return strv; - if (!skip_empty && !skip_repeated) + if (!no_empty && !no_duplicates) return strv; j = 0; for (i = 0; strv[i]; i++) { - if ((skip_empty && !*strv[i]) || (skip_repeated && nm_strv_contains(strv, j, strv[i]))) + if ((no_empty && !*strv[i]) || (no_duplicates && nm_strv_contains(strv, j, strv[i]))) continue; strv[j++] = strv[i]; } @@ -2098,7 +2098,7 @@ nm_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repe } char ** -nm_strv_cleanup(char **strv, gboolean strip_whitespace, gboolean skip_empty, gboolean skip_repeated) +nm_strv_cleanup(char **strv, gboolean strip_whitespace, gboolean no_empty, gboolean no_duplicates) { gsize i; gsize j; @@ -2112,11 +2112,11 @@ nm_strv_cleanup(char **strv, gboolean strip_whitespace, gboolean skip_empty, gbo for (i = 0; strv[i]; i++) g_strstrip(strv[i]); } - if (!skip_empty && !skip_repeated) + if (!no_empty && !no_duplicates) return strv; j = 0; for (i = 0; strv[i]; i++) { - if ((skip_empty && !*strv[i]) || (skip_repeated && nm_strv_contains(strv, j, strv[i]))) + if ((no_empty && !*strv[i]) || (no_duplicates && nm_strv_contains(strv, j, strv[i]))) g_free(strv[i]); else strv[j++] = strv[i]; diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 2a2312fb90..941b7fd986 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -529,12 +529,10 @@ gssize _nm_strv_find_first(const char *const *list, gssize len, const char *need gboolean nm_strv_has_duplicate(const char *const *list, gssize len, gboolean is_sorted); -const char **nm_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repeated); +const char **nm_strv_cleanup_const(const char **strv, gboolean no_empty, gboolean no_duplicates); -char **nm_strv_cleanup(char **strv, - gboolean strip_whitespace, - gboolean skip_empty, - gboolean skip_repeated); +char ** +nm_strv_cleanup(char **strv, gboolean strip_whitespace, gboolean no_empty, gboolean no_duplicates); gboolean nm_strv_is_same_unordered(const char *const *strv1, gssize len1,