From c47d6b17d580fa1466c5aa89e1c7c4717a676501 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Jun 2023 17:01:01 +0200 Subject: [PATCH] core: replace multiple arguments of nm_match_spec_device() with struct Struct allow named arguments, which seems easier to maintain instead of a function with many arguments. Also, adding a new parameter does not require changes to most of the callers. The real advantage of this is that we encode all the search parameters in one argument. And we can add that argument to _match_section_infos_lookup(), alongside lookup by NMDevice or NMPlatformLink. --- src/core/NetworkManagerUtils.c | 16 +++-- src/core/devices/nm-device.c | 20 +++--- src/core/nm-core-utils.c | 124 +++++++++++++++++---------------- src/core/nm-core-utils.h | 20 +++--- src/core/tests/test-core.c | 21 +++--- 5 files changed, 109 insertions(+), 92 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 9dac44502..94e14852a 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -915,13 +915,15 @@ nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, * It's still useful because of specs like "*" and "except:interface-name:eth0", * which match even in that case. */ m = nm_match_spec_device(specs, - pllink ? pllink->name : NULL, - match_device_type, - pllink ? pllink->driver : NULL, - NULL, - NULL, - NULL, - match_dhcp_plugin); + &((const NMMatchSpecDeviceData){ + .interface_name = pllink ? pllink->name : NULL, + .device_type = match_device_type, + .driver = pllink ? pllink->driver : NULL, + .driver_version = NULL, + .hwaddr = NULL, + .s390_subchannels = NULL, + .dhcp_plugin = match_dhcp_plugin, + })); return nm_match_spec_match_type_to_bool(m, no_match_value); } diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 352bd26e1..aae335691 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17294,14 +17294,18 @@ nm_device_spec_match_list_full(NMDevice *self, const GSList *specs, int no_match !nm_device_get_unmanaged_flags(self, NM_UNMANAGED_PLATFORM_INIT), &is_fake); - m = nm_match_spec_device(specs, - nm_device_get_iface(self), - nm_device_get_type_description(self), - nm_device_get_driver(self), - nm_device_get_driver_version(self), - is_fake ? NULL : hw_address, - klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, - nm_dhcp_manager_get_config(nm_dhcp_manager_get())); + m = nm_match_spec_device( + specs, + &((const NMMatchSpecDeviceData){ + .interface_name = nm_device_get_iface(self), + .device_type = nm_device_get_type_description(self), + .driver = nm_device_get_driver(self), + .driver_version = nm_device_get_driver_version(self), + .hwaddr = is_fake ? NULL : hw_address, + .s390_subchannels = + klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, + .dhcp_plugin = nm_dhcp_manager_get_config(nm_dhcp_manager_get()), + })); return nm_match_spec_match_type_to_bool(m, no_match_value); } diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index e6c4c8cbd..5442efbf8 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1148,25 +1148,26 @@ nm_utils_read_link_absolute(const char *link_file, GError **error) #define MATCH_TAG_CONFIG_ENV "env:" typedef struct { - const char *interface_name; - const char *device_type; - const char *driver; - const char *driver_version; - const char *dhcp_plugin; + /* This struct contains pre-processed data from NMMatchSpecDeviceData so + * we only need to parse it once. */ + const NMMatchSpecDeviceData *data; + const char *device_type; + const char *driver; + const char *driver_version; + const char *dhcp_plugin; struct { - const char *value; - gboolean is_parsed; - guint len; - guint8 bin[_NM_UTILS_HWADDR_LEN_MAX]; + gboolean is_parsed; + guint len; + guint8 bin[_NM_UTILS_HWADDR_LEN_MAX]; } hwaddr; struct { - const char *value; - gboolean is_parsed; - guint32 a; - guint32 b; - guint32 c; + gboolean is_parsed; + gboolean is_good; + guint32 a; + guint32 b; + guint32 c; } s390_subchannels; -} MatchDeviceData; +} MatchSpecDeviceData; static gboolean match_device_s390_subchannels_parse(const char *s390_subchannels, @@ -1234,22 +1235,25 @@ match_device_s390_subchannels_parse(const char *s390_subchannels, } static gboolean -match_data_s390_subchannels_eval(const char *spec_str, MatchDeviceData *match_data) +match_data_s390_subchannels_eval(const char *spec_str, MatchSpecDeviceData *match_data) { - guint32 a, b, c; + guint32 a; + guint32 b; + guint32 c; if (G_UNLIKELY(!match_data->s390_subchannels.is_parsed)) { + nm_assert(!match_data->s390_subchannels.is_good); match_data->s390_subchannels.is_parsed = TRUE; - if (!match_data->s390_subchannels.value - || !match_device_s390_subchannels_parse(match_data->s390_subchannels.value, + if (!match_data->data->s390_subchannels + || !match_device_s390_subchannels_parse(match_data->data->s390_subchannels, &match_data->s390_subchannels.a, &match_data->s390_subchannels.b, &match_data->s390_subchannels.c)) { - match_data->s390_subchannels.value = NULL; return FALSE; } - } else if (!match_data->s390_subchannels.value) + match_data->s390_subchannels.is_good = TRUE; + } else if (!match_data->s390_subchannels.is_good) return FALSE; if (!match_device_s390_subchannels_parse(spec_str, &a, &b, &c)) @@ -1259,15 +1263,16 @@ match_data_s390_subchannels_eval(const char *spec_str, MatchDeviceData *match_da } static gboolean -match_device_hwaddr_eval(const char *spec_str, MatchDeviceData *match_data) +match_device_hwaddr_eval(const char *spec_str, MatchSpecDeviceData *match_data) { if (G_UNLIKELY(!match_data->hwaddr.is_parsed)) { match_data->hwaddr.is_parsed = TRUE; + nm_assert(match_data->hwaddr.len == 0); - if (match_data->hwaddr.value) { + if (match_data->data->hwaddr) { gsize l; - if (!_nm_utils_hwaddr_aton(match_data->hwaddr.value, + if (!_nm_utils_hwaddr_aton(match_data->data->hwaddr, match_data->hwaddr.bin, sizeof(match_data->hwaddr.bin), &l)) @@ -1275,7 +1280,7 @@ match_device_hwaddr_eval(const char *spec_str, MatchDeviceData *match_data) match_data->hwaddr.len = l; } else return FALSE; - } else if (!match_data->hwaddr.len) + } else if (match_data->hwaddr.len == 0) return FALSE; return nm_utils_hwaddr_matches(spec_str, -1, match_data->hwaddr.bin, match_data->hwaddr.len); @@ -1330,7 +1335,7 @@ match_except(const char *spec_str, gboolean *out_except) } static gboolean -match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *match_data) +match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchSpecDeviceData *match_data) { if (spec_str[0] == '*' && spec_str[1] == '\0') return TRUE; @@ -1353,10 +1358,10 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m use_pattern = TRUE; } - if (match_data->interface_name) { - if (nm_streq(spec_str, match_data->interface_name)) + if (match_data->data->interface_name) { + if (nm_streq(spec_str, match_data->data->interface_name)) return TRUE; - if (use_pattern && g_pattern_match_simple(spec_str, match_data->interface_name)) + if (use_pattern && g_pattern_match_simple(spec_str, match_data->data->interface_name)) return TRUE; } return FALSE; @@ -1402,7 +1407,8 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m if (allow_fuzzy) { if (match_device_hwaddr_eval(spec_str, match_data)) return TRUE; - if (match_data->interface_name && nm_streq(spec_str, match_data->interface_name)) + if (match_data->data->interface_name + && nm_streq(spec_str, match_data->data->interface_name)) return TRUE; } @@ -1410,42 +1416,40 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m } NMMatchSpecMatchType -nm_match_spec_device(const GSList *specs, - const char *interface_name, - const char *device_type, - const char *driver, - const char *driver_version, - const char *hwaddr, - const char *s390_subchannels, - const char *dhcp_plugin) +nm_match_spec_device(const GSList *specs, const NMMatchSpecDeviceData *data) { - const GSList *iter; - gboolean has_match = FALSE; - gboolean has_match_except = FALSE; - gboolean has_except = FALSE; - gboolean has_not_except = FALSE; - const char *spec_str; - MatchDeviceData match_data = { - .interface_name = interface_name, - .device_type = nm_str_not_empty(device_type), - .driver = nm_str_not_empty(driver), - .driver_version = nm_str_not_empty(driver_version), - .dhcp_plugin = nm_str_not_empty(dhcp_plugin), - .hwaddr = - { - .value = hwaddr, - }, - .s390_subchannels = - { - .value = s390_subchannels, - }, - }; + const GSList *iter; + gboolean has_match = FALSE; + gboolean has_match_except = FALSE; + gboolean has_except = FALSE; + gboolean has_not_except = FALSE; + const char *spec_str; + MatchSpecDeviceData match_data; - nm_assert(!hwaddr || nm_utils_hwaddr_valid(hwaddr, -1)); + nm_assert(data); + nm_assert(!data->hwaddr || nm_utils_hwaddr_valid(data->hwaddr, -1)); if (!specs) return NM_MATCH_SPEC_NO_MATCH; + match_data = (MatchSpecDeviceData){ + .data = data, + .device_type = nm_str_not_empty(data->device_type), + .driver = nm_str_not_empty(data->driver), + .driver_version = nm_str_not_empty(data->driver_version), + .dhcp_plugin = nm_str_not_empty(data->dhcp_plugin), + .hwaddr = + { + .is_parsed = FALSE, + .len = 0, + }, + .s390_subchannels = + { + .is_parsed = FALSE, + .is_good = FALSE, + }, + }; + for (iter = specs; iter; iter = iter->next) { gboolean except; diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index bdcaa9b6d..551125042 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -195,14 +195,18 @@ typedef enum { int nm_match_spec_match_type_to_bool(NMMatchSpecMatchType m, int no_match_value); -NMMatchSpecMatchType nm_match_spec_device(const GSList *specs, - const char *interface_name, - const char *device_type, - const char *driver, - const char *driver_version, - const char *hwaddr, - const char *s390_subchannels, - const char *dhcp_plugin); +typedef struct _NMMatchSpecDeviceData { + const char *interface_name; + const char *device_type; + const char *driver; + const char *driver_version; + const char *dhcp_plugin; + const char *hwaddr; + const char *s390_subchannels; +} NMMatchSpecDeviceData; + +NMMatchSpecMatchType nm_match_spec_device(const GSList *specs, const NMMatchSpecDeviceData *data); + NMMatchSpecMatchType nm_match_spec_config(const GSList *specs, guint nm_version, const char *env); GSList *nm_match_spec_split(const char *value); char *nm_match_spec_join(GSList *specs); diff --git a/src/core/tests/test-core.c b/src/core/tests/test-core.c index a30cc3bab..8296d7457 100644 --- a/src/core/tests/test-core.c +++ b/src/core/tests/test-core.c @@ -1244,13 +1244,9 @@ _test_match_spec_device(const GSList *specs, const char *match_str) { if (match_str && g_str_has_prefix(match_str, MATCH_S390)) return nm_match_spec_device(specs, - NULL, - NULL, - NULL, - NULL, - NULL, - &match_str[NM_STRLEN(MATCH_S390)], - NULL); + &((const NMMatchSpecDeviceData){ + .s390_subchannels = &match_str[NM_STRLEN(MATCH_S390)], + })); if (match_str && g_str_has_prefix(match_str, MATCH_DRIVER)) { gs_free char *s = g_strdup(&match_str[NM_STRLEN(MATCH_DRIVER)]); char *t; @@ -1260,9 +1256,16 @@ _test_match_spec_device(const GSList *specs, const char *match_str) t[0] = '\0'; t++; } - return nm_match_spec_device(specs, NULL, NULL, s, t, NULL, NULL, NULL); + return nm_match_spec_device(specs, + &((const NMMatchSpecDeviceData){ + .driver = s, + .driver_version = t, + })); } - return nm_match_spec_device(specs, match_str, NULL, NULL, NULL, NULL, NULL, NULL); + return nm_match_spec_device(specs, + &((const NMMatchSpecDeviceData){ + .interface_name = match_str, + })); } static void