From 2628c11a17a57276cb50488ecbb5f2b6cf41a470 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. (cherry picked from commit c47d6b17d580fa1466c5aa89e1c7c4717a676501) --- 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 5744a87e2c..42ed16d5f5 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -911,13 +911,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 5d18289b52..2413d1e508 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17050,14 +17050,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 7bba770f84..69ae6ac604 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1154,25 +1154,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, @@ -1240,22 +1241,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)) @@ -1265,15 +1269,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)) @@ -1281,7 +1286,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); @@ -1336,7 +1341,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; @@ -1359,10 +1364,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; @@ -1408,7 +1413,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; } @@ -1416,42 +1422,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 a57786479f..3d3b1260ff 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 887803bffe..dcea4453a9 100644 --- a/src/core/tests/test-core.c +++ b/src/core/tests/test-core.c @@ -1243,13 +1243,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; @@ -1259,9 +1255,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