From 9f9a89d7781dbb4fba851cfcd997cf19601e4adc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2023 17:55:51 +0100 Subject: [PATCH 01/11] glib-aux: cleanup assertions for GArray element size in nm_strvarray helpers The check "sizeof(const char *const *) == g_array_get_element_size((GArray *) strv)" is wrong, but probably harmless, because most likely on our supported architectures all pointer sizes are the same size. Also, just use `sizeof(char *)` instead of `sizeof(const char *)`. Not that it matters, but the GArray holds pointers of `char *`. Also, consistently place the "sizeof()" on the left side of the comparison. --- src/libnm-glib-aux/nm-shared-utils.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index c7278d1ac..954280d5c 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -2996,7 +2996,7 @@ nm_strvarray_ensure(GArray **p) *p = g_array_new(TRUE, FALSE, sizeof(char *)); g_array_set_clear_func(*p, nm_indirect_g_free); } else - nm_assert(g_array_get_element_size(*p) == sizeof(char *)); + nm_assert(sizeof(char *) == g_array_get_element_size(*p)); return *p; } @@ -3007,7 +3007,7 @@ nm_strvarray_add(GArray *array, const char *str) char *s; nm_assert(array); - nm_assert(g_array_get_element_size(array) == sizeof(char *)); + nm_assert(sizeof(char *) == g_array_get_element_size(array)); s = g_strdup(str); g_array_append_val(array, s); @@ -3036,7 +3036,7 @@ nm_strvarray_get_idx(GArray *array, guint idx) static inline const char *const * nm_strvarray_get_strv_non_empty(GArray *arr, guint *length) { - nm_assert(!arr || g_array_get_element_size(arr) == sizeof(char *)); + nm_assert(!arr || sizeof(char *) == g_array_get_element_size(arr)); if (!arr || arr->len == 0) { NM_SET_OUT(length, 0); @@ -3052,7 +3052,7 @@ nm_strvarray_get_strv_non_empty_dup(GArray *arr, guint *length) { const char *const *strv; - nm_assert(!arr || g_array_get_element_size(arr) == sizeof(char *)); + nm_assert(!arr || sizeof(char *) == g_array_get_element_size(arr)); if (!arr || arr->len == 0) { NM_SET_OUT(length, 0); @@ -3072,7 +3072,7 @@ nm_strvarray_get_strv(GArray **arr, guint *length) return (const char *const *) arr; } - nm_assert(g_array_get_element_size(*arr) == sizeof(char *)); + nm_assert(sizeof(char *) == g_array_get_element_size(*arr)); NM_SET_OUT(length, (*arr)->len); return &g_array_index(*arr, const char *, 0); @@ -3085,7 +3085,7 @@ nm_strvarray_set_strv(GArray **array, const char *const *strv) array_old = g_steal_pointer(array); - nm_assert(!array_old || g_array_get_element_size(array_old) == sizeof(char *)); + nm_assert(!array_old || sizeof(char *) == g_array_get_element_size(array_old)); if (!strv || !strv[0]) return; @@ -3103,7 +3103,7 @@ nm_strvarray_find_first(GArray *strv, const char *needle) nm_assert(needle); if (strv) { - nm_assert(g_array_get_element_size(strv) == sizeof(char *)); + nm_assert(sizeof(char *) == g_array_get_element_size(strv)); for (i = 0; i < strv->len; i++) { if (nm_streq(needle, g_array_index(strv, const char *, i))) return i; @@ -3129,8 +3129,8 @@ nm_strvarray_remove_first(GArray *strv, const char *needle) static inline int nm_strvarray_cmp(const GArray *a, const GArray *b) { - nm_assert(!a || sizeof(const char *const *) == g_array_get_element_size((GArray *) a)); - nm_assert(!b || sizeof(const char *const *) == g_array_get_element_size((GArray *) b)); + nm_assert(!a || sizeof(char *) == g_array_get_element_size((GArray *) a)); + nm_assert(!b || sizeof(char *) == g_array_get_element_size((GArray *) b)); NM_CMP_SELF(a, b); @@ -3142,7 +3142,7 @@ nm_strvarray_cmp(const GArray *a, const GArray *b) static inline int _nm_strvarray_cmp_strv(const GArray *strv, const char *const *ss, gsize ss_len) { - nm_assert(!strv || sizeof(const char *const *) == g_array_get_element_size((GArray *) strv)); + nm_assert(!strv || sizeof(char *) == g_array_get_element_size((GArray *) strv)); return nm_strv_cmp_n(nm_g_array_data(strv), strv ? ((gssize) strv->len) : -1, ss, ss_len); } From 7ab9a2b69f531f16d87771b65352eaa1fde827ae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:26:43 +0200 Subject: [PATCH 02/11] glib-aux: add nm_strvarray_contains() helper --- src/libnm-glib-aux/nm-shared-utils.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 954280d5c..5c7ea9571 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3112,6 +3112,8 @@ nm_strvarray_find_first(GArray *strv, const char *needle) return -1; } +#define nm_strvarray_contains(strv, needle) (nm_strvarray_find_first((strv), (needle)) >= 0) + static inline gboolean nm_strvarray_remove_first(GArray *strv, const char *needle) { From 73947cdfd011041b9d9a591ff77320620df2882f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:33:44 +0200 Subject: [PATCH 03/11] glib-aux: add nm_strvarray_clear() helper --- src/libnm-glib-aux/nm-shared-utils.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 5c7ea9571..f27f6f7d7 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3154,6 +3154,24 @@ _nm_strvarray_cmp_strv(const GArray *strv, const char *const *ss, gsize ss_len) #define nm_strvarray_equal_strv(strv, ss, ss_len) \ (nm_strvarray_cmp_strv((strv), (ss), (ss_len)) == 0) +static inline gboolean +nm_strvarray_clear(GArray **array) +{ + gboolean cleared = FALSE; + + nm_assert(array); + nm_assert(!*array || sizeof(char *) == g_array_get_element_size(*array)); + + if (*array) { + /* We always clear the GArray, but we return TRUE only if the + * array was non-empty before. */ + if ((*array)->len > 0) + cleared = TRUE; + nm_clear_pointer(array, g_array_unref); + } + return cleared; +} + /*****************************************************************************/ struct _NMVariantAttributeSpec { From 6c83f7bd67916c67071e6252f51f7aa37b3f675f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:39:43 +0200 Subject: [PATCH 04/11] glib-aux: add nm_strvarray_ensure_and_add() helper --- src/libnm-glib-aux/nm-shared-utils.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index f27f6f7d7..9d98e86f4 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3128,6 +3128,23 @@ nm_strvarray_remove_first(GArray *strv, const char *needle) return TRUE; } +static inline void +nm_strvarray_ensure_and_add(GArray **p, const char *str) +{ + nm_strvarray_add(nm_strvarray_ensure(p), str); +} + +static inline gboolean +nm_strvarray_ensure_and_add_unique(GArray **p, const char *str) +{ + nm_assert(p); + + if (nm_strvarray_contains(*p, str)) + return FALSE; + nm_strvarray_add(nm_strvarray_ensure(p), str); + return TRUE; +} + static inline int nm_strvarray_cmp(const GArray *a, const GArray *b) { From 60375218d1389765f7791f1716c8eddbf1935f9d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2023 11:56:23 +0100 Subject: [PATCH 05/11] glib-aux: add nm_strvarray_remove_index() helper --- src/libnm-glib-aux/nm-shared-utils.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 9d98e86f4..90e10c13e 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3128,6 +3128,20 @@ nm_strvarray_remove_first(GArray *strv, const char *needle) return TRUE; } +#define nm_strvarray_remove_index(strv, idx) \ + G_STMT_START \ + { \ + GArray *const _strv = (strv); \ + typeof(idx) _idx = (idx); \ + \ + nm_assert(_strv); \ + nm_assert((uintmax_t) _idx < _strv->len); \ + nm_assert(sizeof(char *) == g_array_get_element_size(_strv)); \ + \ + g_array_remove_index(_strv, (guint) _idx); \ + } \ + G_STMT_END + static inline void nm_strvarray_ensure_and_add(GArray **p, const char *str) { From 2d8c4cfe05dbff74a7342724e807955e95bf1588 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2023 16:08:58 +0100 Subject: [PATCH 06/11] glib-aux: add nm_strvarray_add_take() helper --- src/libnm-glib-aux/nm-shared-utils.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 90e10c13e..eb7d48cfd 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3002,15 +3002,18 @@ nm_strvarray_ensure(GArray **p) } static inline void -nm_strvarray_add(GArray *array, const char *str) +nm_strvarray_add_take(GArray *array, char *str) { - char *s; - nm_assert(array); nm_assert(sizeof(char *) == g_array_get_element_size(array)); - s = g_strdup(str); - g_array_append_val(array, s); + g_array_append_val(array, str); +} + +static inline void +nm_strvarray_add(GArray *array, const char *str) +{ + nm_strvarray_add_take(array, g_strdup(str)); } static inline const char * From 7b5e8381f087809e6cdcd0e88b274d186c75266b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2023 16:09:24 +0100 Subject: [PATCH 07/11] glib-aux: assert against NULL arguments for nm_strvarray_add() --- src/libnm-glib-aux/nm-shared-utils.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index eb7d48cfd..914a3283e 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3007,6 +3007,10 @@ nm_strvarray_add_take(GArray *array, char *str) nm_assert(array); nm_assert(sizeof(char *) == g_array_get_element_size(array)); + /* The array is used as a NULL terminated strv array. Adding NULL is most + * likely a bug. Assert against it. */ + nm_assert(str); + g_array_append_val(array, str); } From 3435bc3011d5c6c4643ce71531de0c28c79f165a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:27:07 +0200 Subject: [PATCH 08/11] libnm: move NMValueStrv definition in header --- src/libnm-core-impl/nm-setting-private.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index bdb21480a..70b19ee2a 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -20,6 +20,14 @@ /*****************************************************************************/ +/* This holds a property of type NM_VALUE_TYPE_STRV. You probably want + * to use nm_strvarray_*() API with this. */ +typedef struct { + GArray *arr; +} NMValueStrv; + +/*****************************************************************************/ + struct _NMRefString; typedef struct { @@ -277,14 +285,6 @@ gboolean _nm_setting_clear_secrets(NMSetting *setting, /*****************************************************************************/ -/* This holds a property of type NM_VALUE_TYPE_STRV. You probably want - * to use nm_strvarray_*() API with this. */ -typedef struct { - GArray *arr; -} NMValueStrv; - -/*****************************************************************************/ - struct _NMRange { int refcount; guint64 start; From eed4a21fa3fa8dd9239c2a0598cfb51370aa9283 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:37:34 +0200 Subject: [PATCH 09/11] libnm: use nm_strvarray_*() helpers for strv properties We have many properties, and we aim that they have a small set of "types". The purpose is that we can treat similar properties (with the same type) alike. One type are "direct" strv properties. Those still require some C functions, like get-length(), clear(), add(), get-at-index(). The implementation of those functions should also be similar, so that strv properties behave similar. For that, make use of helper functions, so that little duplicate logic is there. Use some new nm_strvarray_*() functions, and unify/cleanup some code. All related to strv properties in NMSetting classes. --- src/libnm-core-impl/nm-setting-connection.c | 15 +++-- src/libnm-core-impl/nm-setting-ip-config.c | 2 +- src/libnm-core-impl/nm-setting-match.c | 68 +++++++++------------ 3 files changed, 36 insertions(+), 49 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 98870c731..a228ecf0c 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -881,10 +881,9 @@ nm_setting_connection_add_secondary(NMSettingConnection *setting, const char *se priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting); - if (nm_strvarray_find_first(priv->secondaries.arr, sec_uuid) >= 0) + if (!nm_strvarray_ensure_and_add_unique(&priv->secondaries.arr, sec_uuid)) return FALSE; - nm_strvarray_add(nm_strvarray_ensure(&priv->secondaries.arr), sec_uuid); _notify(setting, PROP_SECONDARIES); return TRUE; } @@ -907,7 +906,7 @@ nm_setting_connection_remove_secondary(NMSettingConnection *setting, guint32 idx g_return_if_fail(idx < nm_g_array_len(priv->secondaries.arr)); - g_array_remove_index(priv->secondaries.arr, idx); + nm_strvarray_remove_index(priv->secondaries.arr, idx); _notify(setting, PROP_SECONDARIES); } @@ -930,11 +929,11 @@ nm_setting_connection_remove_secondary_by_value(NMSettingConnection *setting, co priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting); - if (nm_strvarray_remove_first(priv->secondaries.arr, sec_uuid)) { - _notify(setting, PROP_SECONDARIES); - return TRUE; - } - return FALSE; + if (!nm_strvarray_remove_first(priv->secondaries.arr, sec_uuid)) + return FALSE; + + _notify(setting, PROP_SECONDARIES); + return TRUE; } /** diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index c305e48e9..3f57213a2 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -5385,7 +5385,7 @@ nm_setting_ip_config_add_dhcp_reject_server(NMSettingIPConfig *setting, const ch g_return_if_fail(server != NULL); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - nm_strvarray_add(nm_strvarray_ensure(&priv->dhcp_reject_servers), server); + nm_strvarray_ensure_and_add(&priv->dhcp_reject_servers, server); _notify(setting, PROP_DHCP_REJECT_SERVERS); } diff --git a/src/libnm-core-impl/nm-setting-match.c b/src/libnm-core-impl/nm-setting-match.c index c35e38712..4cad4f68a 100644 --- a/src/libnm-core-impl/nm-setting-match.c +++ b/src/libnm-core-impl/nm-setting-match.c @@ -97,7 +97,7 @@ nm_setting_match_add_interface_name(NMSettingMatch *setting, const char *interfa g_return_if_fail(NM_IS_SETTING_MATCH(setting)); g_return_if_fail(interface_name); - nm_strvarray_add(nm_strvarray_ensure(&setting->interface_name.arr), interface_name); + nm_strvarray_ensure_and_add(&setting->interface_name.arr, interface_name); _notify(setting, PROP_INTERFACE_NAME); } @@ -118,7 +118,7 @@ nm_setting_match_remove_interface_name(NMSettingMatch *setting, int idx) g_return_if_fail(setting->interface_name.arr && idx >= 0 && idx < setting->interface_name.arr->len); - g_array_remove_index(setting->interface_name.arr, idx); + nm_strvarray_remove_index(setting->interface_name.arr, idx); _notify(setting, PROP_INTERFACE_NAME); } @@ -139,12 +139,11 @@ nm_setting_match_remove_interface_name_by_value(NMSettingMatch *setting, const c g_return_val_if_fail(NM_IS_SETTING_MATCH(setting), FALSE); g_return_val_if_fail(interface_name, FALSE); - if (nm_strvarray_remove_first(setting->interface_name.arr, interface_name)) { - _notify(setting, PROP_INTERFACE_NAME); - return TRUE; - } + if (!nm_strvarray_remove_first(setting->interface_name.arr, interface_name)) + return FALSE; - return FALSE; + _notify(setting, PROP_INTERFACE_NAME); + return TRUE; } /** @@ -160,10 +159,8 @@ nm_setting_match_clear_interface_names(NMSettingMatch *setting) { g_return_if_fail(NM_IS_SETTING_MATCH(setting)); - if (nm_g_array_len(setting->interface_name.arr) != 0) { - nm_clear_pointer(&setting->interface_name.arr, g_array_unref); + if (nm_strvarray_clear(&setting->interface_name.arr)) _notify(setting, PROP_INTERFACE_NAME); - } } /** @@ -240,7 +237,7 @@ nm_setting_match_add_kernel_command_line(NMSettingMatch *setting, const char *ke g_return_if_fail(NM_IS_SETTING_MATCH(setting)); g_return_if_fail(kernel_command_line); - nm_strvarray_add(nm_strvarray_ensure(&setting->kernel_command_line.arr), kernel_command_line); + nm_strvarray_ensure_and_add(&setting->kernel_command_line.arr, kernel_command_line); _notify(setting, PROP_KERNEL_COMMAND_LINE); } @@ -261,7 +258,7 @@ nm_setting_match_remove_kernel_command_line(NMSettingMatch *setting, guint idx) g_return_if_fail(setting->kernel_command_line.arr && idx < setting->kernel_command_line.arr->len); - g_array_remove_index(setting->kernel_command_line.arr, idx); + nm_strvarray_remove_index(setting->kernel_command_line.arr, idx); _notify(setting, PROP_KERNEL_COMMAND_LINE); } @@ -283,12 +280,11 @@ nm_setting_match_remove_kernel_command_line_by_value(NMSettingMatch *setting, g_return_val_if_fail(NM_IS_SETTING_MATCH(setting), FALSE); g_return_val_if_fail(kernel_command_line, FALSE); - if (nm_strvarray_remove_first(setting->kernel_command_line.arr, kernel_command_line)) { - _notify(setting, PROP_KERNEL_COMMAND_LINE); - return TRUE; - } + if (!nm_strvarray_remove_first(setting->kernel_command_line.arr, kernel_command_line)) + return FALSE; - return FALSE; + _notify(setting, PROP_KERNEL_COMMAND_LINE); + return TRUE; } /** @@ -304,10 +300,8 @@ nm_setting_match_clear_kernel_command_lines(NMSettingMatch *setting) { g_return_if_fail(NM_IS_SETTING_MATCH(setting)); - if (nm_g_array_len(setting->kernel_command_line.arr) != 0) { - nm_clear_pointer(&setting->kernel_command_line.arr, g_array_unref); + if (nm_strvarray_clear(&setting->kernel_command_line.arr)) _notify(setting, PROP_KERNEL_COMMAND_LINE); - } } /** @@ -381,7 +375,7 @@ nm_setting_match_add_driver(NMSettingMatch *setting, const char *driver) g_return_if_fail(NM_IS_SETTING_MATCH(setting)); g_return_if_fail(driver); - nm_strvarray_add(nm_strvarray_ensure(&setting->driver.arr), driver); + nm_strvarray_ensure_and_add(&setting->driver.arr, driver); _notify(setting, PROP_DRIVER); } @@ -401,7 +395,7 @@ nm_setting_match_remove_driver(NMSettingMatch *setting, guint idx) g_return_if_fail(setting->driver.arr && idx < setting->driver.arr->len); - g_array_remove_index(setting->driver.arr, idx); + nm_strvarray_remove_index(setting->driver.arr, idx); _notify(setting, PROP_DRIVER); } @@ -422,12 +416,11 @@ nm_setting_match_remove_driver_by_value(NMSettingMatch *setting, const char *dri g_return_val_if_fail(NM_IS_SETTING_MATCH(setting), FALSE); g_return_val_if_fail(driver, FALSE); - if (nm_strvarray_remove_first(setting->driver.arr, driver)) { - _notify(setting, PROP_DRIVER); - return TRUE; - } + if (!nm_strvarray_remove_first(setting->driver.arr, driver)) + return FALSE; - return FALSE; + _notify(setting, PROP_DRIVER); + return TRUE; } /** @@ -443,10 +436,8 @@ nm_setting_match_clear_drivers(NMSettingMatch *setting) { g_return_if_fail(NM_IS_SETTING_MATCH(setting)); - if (nm_g_array_len(setting->driver.arr) != 0) { - nm_clear_pointer(&setting->driver.arr, g_array_unref); + if (nm_strvarray_clear(&setting->driver.arr)) _notify(setting, PROP_DRIVER); - } } /** @@ -520,7 +511,7 @@ nm_setting_match_add_path(NMSettingMatch *setting, const char *path) g_return_if_fail(NM_IS_SETTING_MATCH(setting)); g_return_if_fail(path); - nm_strvarray_add(nm_strvarray_ensure(&setting->path.arr), path); + nm_strvarray_ensure_and_add(&setting->path.arr, path); _notify(setting, PROP_PATH); } @@ -540,7 +531,7 @@ nm_setting_match_remove_path(NMSettingMatch *setting, guint idx) g_return_if_fail(setting->path.arr && idx < setting->path.arr->len); - g_array_remove_index(setting->path.arr, idx); + nm_strvarray_remove_index(setting->path.arr, idx); _notify(setting, PROP_PATH); } @@ -561,12 +552,11 @@ nm_setting_match_remove_path_by_value(NMSettingMatch *setting, const char *path) g_return_val_if_fail(NM_IS_SETTING_MATCH(setting), FALSE); g_return_val_if_fail(path, FALSE); - if (nm_strvarray_remove_first(setting->path.arr, path)) { - _notify(setting, PROP_PATH); - return TRUE; - } + if (!nm_strvarray_remove_first(setting->path.arr, path)) + return FALSE; - return FALSE; + _notify(setting, PROP_PATH); + return TRUE; } /** @@ -582,10 +572,8 @@ nm_setting_match_clear_paths(NMSettingMatch *setting) { g_return_if_fail(NM_IS_SETTING_MATCH(setting)); - if (nm_g_array_len(setting->path.arr) != 0) { - nm_clear_pointer(&setting->path.arr, g_array_unref); + if (nm_strvarray_clear(&setting->path.arr)) _notify(setting, PROP_PATH); - } } /** From 4cd58207c133ce81257dacb42d2181367b962ac3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:27:36 +0200 Subject: [PATCH 10/11] libnm: implement "ipv4.dns-search" as direct-strv property --- src/libnm-core-impl/nm-setting-ip-config.c | 72 ++++++++-------------- src/libnm-core-impl/nm-setting-private.h | 50 +++++++-------- src/libnm-core-impl/tests/test-general.c | 18 +++++- 3 files changed, 68 insertions(+), 72 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 3f57213a2..e4163b4c6 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -4231,7 +4231,7 @@ nm_setting_ip_config_get_num_dns_searches(NMSettingIPConfig *setting) { g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), 0); - return NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dns_search->len; + return nm_g_array_len(NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dns_search.arr); } /** @@ -4239,19 +4239,18 @@ nm_setting_ip_config_get_num_dns_searches(NMSettingIPConfig *setting) * @setting: the #NMSettingIPConfig * @idx: index number of the DNS search domain to return * + * Since 1.46, access at index "len" is allowed and returns NULL. + * * Returns: the DNS search domain at index @idx **/ const char * nm_setting_ip_config_get_dns_search(NMSettingIPConfig *setting, int idx) { - NMSettingIPConfigPrivate *priv; - g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), NULL); - priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - g_return_val_if_fail(idx >= 0 && idx < priv->dns_search->len, NULL); - - return priv->dns_search->pdata[idx]; + return nm_strvarray_get_idxnull_or_greturn( + NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dns_search.arr, + idx); } /** @@ -4268,19 +4267,16 @@ gboolean nm_setting_ip_config_add_dns_search(NMSettingIPConfig *setting, const char *dns_search) { NMSettingIPConfigPrivate *priv; - guint i; g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE); g_return_val_if_fail(dns_search != NULL, FALSE); g_return_val_if_fail(dns_search[0] != '\0', FALSE); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - for (i = 0; i < priv->dns_search->len; i++) { - if (!strcmp(dns_search, priv->dns_search->pdata[i])) - return FALSE; - } - g_ptr_array_add(priv->dns_search, g_strdup(dns_search)); + if (!nm_strvarray_ensure_and_add_unique(&priv->dns_search.arr, dns_search)) + return FALSE; + _notify(setting, PROP_DNS_SEARCH); return TRUE; } @@ -4300,9 +4296,10 @@ nm_setting_ip_config_remove_dns_search(NMSettingIPConfig *setting, int idx) g_return_if_fail(NM_IS_SETTING_IP_CONFIG(setting)); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - g_return_if_fail(idx >= 0 && idx < priv->dns_search->len); - g_ptr_array_remove_index(priv->dns_search, idx); + g_return_if_fail(idx >= 0 && idx < nm_g_array_len(priv->dns_search.arr)); + + nm_strvarray_remove_index(priv->dns_search.arr, idx); _notify(setting, PROP_DNS_SEARCH); } @@ -4319,21 +4316,18 @@ gboolean nm_setting_ip_config_remove_dns_search_by_value(NMSettingIPConfig *setting, const char *dns_search) { NMSettingIPConfigPrivate *priv; - guint i; g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE); g_return_val_if_fail(dns_search != NULL, FALSE); g_return_val_if_fail(dns_search[0] != '\0', FALSE); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - for (i = 0; i < priv->dns_search->len; i++) { - if (!strcmp(dns_search, priv->dns_search->pdata[i])) { - g_ptr_array_remove_index(priv->dns_search, i); - _notify(setting, PROP_DNS_SEARCH); - return TRUE; - } - } - return FALSE; + + if (!nm_strvarray_remove_first(priv->dns_search.arr, dns_search)) + return FALSE; + + _notify(setting, PROP_DNS_SEARCH); + return TRUE; } /** @@ -4345,16 +4339,10 @@ nm_setting_ip_config_remove_dns_search_by_value(NMSettingIPConfig *setting, cons void nm_setting_ip_config_clear_dns_searches(NMSettingIPConfig *setting) { - NMSettingIPConfigPrivate *priv; - g_return_if_fail(NM_IS_SETTING_IP_CONFIG(setting)); - priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - - if (priv->dns_search->len != 0) { - g_ptr_array_set_size(priv->dns_search, 0); + if (nm_strvarray_clear(&NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dns_search.arr)) _notify(setting, PROP_DNS_SEARCH); - } } /** @@ -6132,9 +6120,12 @@ _nm_sett_info_property_override_create_array_ip_config(int addr_family) .direct_offset = NM_STRUCT_OFFSET_ENSURE_TYPE(int, NMSettingIPConfigPrivate, replace_local_rule)); - _nm_properties_override_gobj(properties_override, - obj_properties[PROP_DNS_SEARCH], - &nm_sett_info_propert_type_gprop_strv_oldstyle); + _nm_properties_override_gobj( + properties_override, + obj_properties[PROP_DNS_SEARCH], + &nm_sett_info_propert_type_direct_strv, + .direct_offset = + NM_STRUCT_OFFSET_ENSURE_TYPE(NMValueStrv, NMSettingIPConfigPrivate, dns_search)); _nm_properties_override_gobj(properties_override, obj_properties[PROP_DNS_OPTIONS], @@ -6159,9 +6150,6 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) case PROP_DNS: g_value_take_boxed(value, _nm_utils_ptrarray_to_strv(priv->dns)); break; - case PROP_DNS_SEARCH: - g_value_take_boxed(value, _nm_utils_ptrarray_to_strv(priv->dns_search)); - break; case PROP_DNS_OPTIONS: g_value_take_boxed(value, priv->dns_options ? _nm_utils_ptrarray_to_strv(priv->dns_options) @@ -6209,10 +6197,6 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps } break; } - case PROP_DNS_SEARCH: - g_ptr_array_unref(priv->dns_search); - priv->dns_search = nm_strv_to_ptrarray(g_value_get_boxed(value)); - break; case PROP_DNS_OPTIONS: strv = g_value_get_boxed(value); if (!strv) { @@ -6260,9 +6244,8 @@ _nm_setting_ip_config_private_init(gpointer self, NMSettingIPConfigPrivate *priv { nm_assert(NM_IS_SETTING_IP_CONFIG(self)); - priv->dns_search = g_ptr_array_new_with_free_func(g_free); - priv->addresses = g_ptr_array_new_with_free_func((GDestroyNotify) nm_ip_address_unref); - priv->routes = g_ptr_array_new_with_free_func((GDestroyNotify) nm_ip_route_unref); + priv->addresses = g_ptr_array_new_with_free_func((GDestroyNotify) nm_ip_address_unref); + priv->routes = g_ptr_array_new_with_free_func((GDestroyNotify) nm_ip_route_unref); } static void @@ -6278,7 +6261,6 @@ finalize(GObject *object) NMSettingIPConfigPrivate *priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(self); nm_g_ptr_array_unref(priv->dns); - g_ptr_array_unref(priv->dns_search); nm_g_ptr_array_unref(priv->dns_options); g_ptr_array_unref(priv->addresses); g_ptr_array_unref(priv->routes); diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 70b19ee2a..646270195 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -174,31 +174,31 @@ struct _NMSettingIPConfigClass { }; typedef struct { - GPtrArray *dns; /* array of IP address strings */ - GPtrArray *dns_search; /* array of domain name strings */ - GPtrArray *dns_options; /* array of DNS options */ - GPtrArray *addresses; /* array of NMIPAddress */ - GPtrArray *routes; /* array of NMIPRoute */ - GPtrArray *routing_rules; - GArray *dhcp_reject_servers; - char *method; - char *gateway; - char *dhcp_hostname; - char *dhcp_iaid; - gint64 route_metric; - int auto_route_ext_gw; - int replace_local_rule; - gint32 required_timeout; - gint32 dad_timeout; - gint32 dhcp_timeout; - gint32 dns_priority; - guint32 route_table; - guint32 dhcp_hostname_flags; - bool ignore_auto_routes; - bool ignore_auto_dns; - bool dhcp_send_hostname; - bool never_default; - bool may_fail; + NMValueStrv dns_search; /* array of domain name strings */ + GPtrArray *dns; /* array of IP address strings */ + GPtrArray *dns_options; /* array of DNS options */ + GPtrArray *addresses; /* array of NMIPAddress */ + GPtrArray *routes; /* array of NMIPRoute */ + GPtrArray *routing_rules; + GArray *dhcp_reject_servers; + char *method; + char *gateway; + char *dhcp_hostname; + char *dhcp_iaid; + gint64 route_metric; + int auto_route_ext_gw; + int replace_local_rule; + gint32 required_timeout; + gint32 dad_timeout; + gint32 dhcp_timeout; + gint32 dns_priority; + guint32 route_table; + guint32 dhcp_hostname_flags; + bool ignore_auto_routes; + bool ignore_auto_dns; + bool dhcp_send_hostname; + bool never_default; + bool may_fail; } NMSettingIPConfigPrivate; void _nm_setting_ip_config_private_init(gpointer self, NMSettingIPConfigPrivate *priv); diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index 0603ea61e..c8952bf3e 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -5335,7 +5335,8 @@ test_setting_ip4_changed_signal(void) ASSERT_CHANGED(nm_setting_ip_config_add_dns_search(s_ip4, "foobar.com")); ASSERT_CHANGED(nm_setting_ip_config_remove_dns_search(s_ip4, 0)); - NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(idx >= 0 && idx < priv->dns_search->len)); + NMTST_EXPECT_LIBNM_CRITICAL( + NMTST_G_RETURN_MSG(idx >= 0 && idx < nm_g_array_len(priv->dns_search.arr))); ASSERT_UNCHANGED(nm_setting_ip_config_remove_dns_search(s_ip4, 1)); g_test_assert_expected_messages(); @@ -5410,9 +5411,22 @@ test_setting_ip6_changed_signal(void) ASSERT_CHANGED(nm_setting_ip_config_clear_dns(s_ip6)); ASSERT_CHANGED(nm_setting_ip_config_add_dns_search(s_ip6, "foobar.com")); + + g_assert_cmpstr(nm_setting_ip_config_get_dns_search(s_ip6, 0), ==, "foobar.com"); + g_assert_cmpstr(nm_setting_ip_config_get_dns_search(s_ip6, 1), ==, NULL); + + NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(_idx <= _len)); + g_assert_cmpstr(nm_setting_ip_config_get_dns_search(s_ip6, -1), ==, NULL); + g_test_assert_expected_messages(); + + NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(_idx <= _len)); + g_assert_cmpstr(nm_setting_ip_config_get_dns_search(s_ip6, 2), ==, NULL); + g_test_assert_expected_messages(); + ASSERT_CHANGED(nm_setting_ip_config_remove_dns_search(s_ip6, 0)); - NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(idx >= 0 && idx < priv->dns_search->len)); + NMTST_EXPECT_LIBNM_CRITICAL( + NMTST_G_RETURN_MSG(idx >= 0 && idx < nm_g_array_len(priv->dns_search.arr))); ASSERT_UNCHANGED(nm_setting_ip_config_remove_dns_search(s_ip6, 1)); g_test_assert_expected_messages(); From 8079e8969df9886feed6aa8e2c38b3b4abf3f351 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 26 Oct 2023 13:27:36 +0200 Subject: [PATCH 11/11] libnm: implement "ipv4.dhcp-reject-servers" as direct-strv property --- src/libnm-core-impl/nm-setting-ip-config.c | 50 ++++++++++------------ src/libnm-core-impl/nm-setting-private.h | 4 +- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index e4163b4c6..06e250dcb 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -5351,8 +5351,10 @@ const char *const * nm_setting_ip_config_get_dhcp_reject_servers(NMSettingIPConfig *setting, guint *out_len) { g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), NULL); - return nm_strvarray_get_strv(&NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dhcp_reject_servers, - out_len); + + return nm_strvarray_get_strv( + &NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dhcp_reject_servers.arr, + out_len); } /** @@ -5367,13 +5369,11 @@ nm_setting_ip_config_get_dhcp_reject_servers(NMSettingIPConfig *setting, guint * void nm_setting_ip_config_add_dhcp_reject_server(NMSettingIPConfig *setting, const char *server) { - NMSettingIPConfigPrivate *priv; - g_return_if_fail(NM_IS_SETTING_IP_CONFIG(setting)); - g_return_if_fail(server != NULL); - priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); + g_return_if_fail(server); - nm_strvarray_ensure_and_add(&priv->dhcp_reject_servers, server); + nm_strvarray_ensure_and_add(&NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dhcp_reject_servers.arr, + server); _notify(setting, PROP_DHCP_REJECT_SERVERS); } @@ -5392,10 +5392,12 @@ nm_setting_ip_config_remove_dhcp_reject_server(NMSettingIPConfig *setting, guint NMSettingIPConfigPrivate *priv; g_return_if_fail(NM_IS_SETTING_IP_CONFIG(setting)); - priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - g_return_if_fail(priv->dhcp_reject_servers && idx < priv->dhcp_reject_servers->len); - g_array_remove_index(priv->dhcp_reject_servers, idx); + priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); + + g_return_if_fail(idx < nm_g_array_len(priv->dhcp_reject_servers.arr)); + + nm_strvarray_remove_index(priv->dhcp_reject_servers.arr, idx); _notify(setting, PROP_DHCP_REJECT_SERVERS); } @@ -5410,15 +5412,10 @@ nm_setting_ip_config_remove_dhcp_reject_server(NMSettingIPConfig *setting, guint void nm_setting_ip_config_clear_dhcp_reject_servers(NMSettingIPConfig *setting) { - NMSettingIPConfigPrivate *priv; - g_return_if_fail(NM_IS_SETTING_IP_CONFIG(setting)); - priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - if (nm_g_array_len(priv->dhcp_reject_servers) != 0) { - nm_clear_pointer(&priv->dhcp_reject_servers, g_array_unref); + if (nm_strvarray_clear(&NM_SETTING_IP_CONFIG_GET_PRIVATE(setting)->dhcp_reject_servers.arr)) _notify(setting, PROP_DHCP_REJECT_SERVERS); - } } /** @@ -5708,7 +5705,7 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) } /* Validate reject servers */ - if (priv->dhcp_reject_servers && priv->dhcp_reject_servers->len != 0) { + if (priv->dhcp_reject_servers.arr && priv->dhcp_reject_servers.arr->len > 0) { if (NM_SETTING_IP_CONFIG_GET_ADDR_FAMILY(setting) != AF_INET) { g_set_error_literal(error, NM_CONNECTION_ERROR, @@ -5721,17 +5718,17 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - for (i = 0; i < priv->dhcp_reject_servers->len; i++) { + for (i = 0; i < priv->dhcp_reject_servers.arr->len; i++) { if (!nm_inet_parse_with_prefix_str( NM_SETTING_IP_CONFIG_GET_ADDR_FAMILY(setting), - nm_g_array_index(priv->dhcp_reject_servers, const char *, i), + nm_g_array_index(priv->dhcp_reject_servers.arr, const char *, i), NULL, NULL)) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, _("'%s' is not a valid IP or subnet"), - nm_g_array_index(priv->dhcp_reject_servers, const char *, i)); + nm_g_array_index(priv->dhcp_reject_servers.arr, const char *, i)); g_prefix_error(error, "%s.%s: ", nm_setting_get_name(setting), @@ -6133,7 +6130,11 @@ _nm_sett_info_property_override_create_array_ip_config(int addr_family) _nm_properties_override_gobj(properties_override, obj_properties[PROP_DHCP_REJECT_SERVERS], - &nm_sett_info_propert_type_gprop_strv_oldstyle); + &nm_sett_info_propert_type_direct_strv, + .direct_offset = + NM_STRUCT_OFFSET_ENSURE_TYPE(NMValueStrv, + NMSettingIPConfigPrivate, + dhcp_reject_servers)); return properties_override; } @@ -6167,9 +6168,6 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) (NMUtilsCopyFunc) nm_ip_route_dup, (GDestroyNotify) nm_ip_route_unref)); break; - case PROP_DHCP_REJECT_SERVERS: - g_value_set_boxed(value, nm_strvarray_get_strv_non_empty(priv->dhcp_reject_servers, NULL)); - break; default: _nm_setting_property_get_property_direct(object, prop_id, value, pspec); break; @@ -6228,9 +6226,6 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps (NMUtilsCopyFunc) nm_ip_route_dup, (GDestroyNotify) nm_ip_route_unref); break; - case PROP_DHCP_REJECT_SERVERS: - nm_strvarray_set_strv(&priv->dhcp_reject_servers, g_value_get_boxed(value)); - break; default: _nm_setting_property_set_property_direct(object, prop_id, value, pspec); break; @@ -6265,7 +6260,6 @@ finalize(GObject *object) g_ptr_array_unref(priv->addresses); g_ptr_array_unref(priv->routes); nm_g_ptr_array_unref(priv->routing_rules); - nm_g_array_unref(priv->dhcp_reject_servers); G_OBJECT_CLASS(nm_setting_ip_config_parent_class)->finalize(object); } diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 646270195..e3f73fec5 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -174,13 +174,13 @@ struct _NMSettingIPConfigClass { }; typedef struct { - NMValueStrv dns_search; /* array of domain name strings */ + NMValueStrv dns_search; /* array of domain name strings */ + NMValueStrv dhcp_reject_servers; GPtrArray *dns; /* array of IP address strings */ GPtrArray *dns_options; /* array of DNS options */ GPtrArray *addresses; /* array of NMIPAddress */ GPtrArray *routes; /* array of NMIPRoute */ GPtrArray *routing_rules; - GArray *dhcp_reject_servers; char *method; char *gateway; char *dhcp_hostname;