From 70c2478f891e257fa8bccfbd115b03d1cc3e48ee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Feb 2022 17:09:53 +0100 Subject: [PATCH] libnm: change NMVariantAttributeSpec.str_type to work for attributes of any type First of all, all of NMVariantAttributeSpec is internal API. We only expose the typedef itself as public API, but not its fields nor their meaning. So we can change things. Change "str_type" to "type_detail", so that it can work for any kind of attribute, not only for strings. Usually, we want to avoid special cases and treat all attributes the same, based on their GVariant type. But sometimes, it is necessary to do something special with an attribute. This is what the "type_detail" encodes, but it's not only relevant for strings. (cherry picked from commit 6f277d8fa6375253774c1bee0d6eef962036734b) --- src/libnm-core-impl/nm-setting-ip-config.c | 122 +++++++++++---------- src/libnm-core-impl/nm-setting-sriov.c | 40 +++---- src/libnm-glib-aux/nm-shared-utils.h | 8 +- 3 files changed, 90 insertions(+), 80 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 9e42bc7d8..88fbe925e 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -1203,8 +1203,8 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = { .v6 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_FROM, G_VARIANT_TYPE_STRING, - .v6 = TRUE, - .str_type = 'p', ), + .v6 = TRUE, + .type_detail = 'p', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_INITCWND, G_VARIANT_TYPE_UINT32, .v4 = TRUE, @@ -1246,9 +1246,9 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = { .v4 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_SRC, G_VARIANT_TYPE_STRING, - .v4 = TRUE, - .v6 = TRUE, - .str_type = 'a', ), + .v4 = TRUE, + .v6 = TRUE, + .type_detail = 'a', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_TABLE, G_VARIANT_TYPE_UINT32, .v4 = TRUE, @@ -1256,9 +1256,9 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = { NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_TOS, G_VARIANT_TYPE_BYTE, .v4 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_TYPE, G_VARIANT_TYPE_STRING, - .v4 = TRUE, - .v6 = TRUE, - .str_type = 'T', ), + .v4 = TRUE, + .v6 = TRUE, + .type_detail = 'T', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_IP_ROUTE_ATTRIBUTE_WINDOW, G_VARIANT_TYPE_UINT32, .v4 = TRUE, @@ -1302,6 +1302,7 @@ nm_ip_route_attribute_validate(const char *name, GError **error) { const NMVariantAttributeSpec *spec; + const char *string; g_return_val_if_fail(name, FALSE); g_return_val_if_fail(value, FALSE); @@ -1340,65 +1341,68 @@ nm_ip_route_attribute_validate(const char *name, return FALSE; } - if (g_variant_type_equal(spec->type, G_VARIANT_TYPE_STRING)) { - const char *string = g_variant_get_string(value, NULL); + switch (spec->type_detail) { + case 'a': /* IP address */ + string = g_variant_get_string(value, NULL); + if (!nm_utils_ipaddr_is_valid(family, string)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + family == AF_INET ? _("'%s' is not a valid IPv4 address") + : _("'%s' is not a valid IPv6 address"), + string); + return FALSE; + } + break; + case 'p': /* IP address + optional prefix */ + { + gs_free char *addr_free = NULL; + const char *addr; + const char *str; - switch (spec->str_type) { - case 'a': /* IP address */ - if (!nm_utils_ipaddr_is_valid(family, string)) { + string = g_variant_get_string(value, NULL); + addr = string; + + str = strchr(addr, '/'); + if (str) { + addr = nm_strndup_a(200, addr, str - addr, &addr_free); + str++; + if (_nm_utils_ascii_str_to_int64(str, 10, 0, family == AF_INET ? 32 : 128, -1) < 0) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, - family == AF_INET ? _("'%s' is not a valid IPv4 address") - : _("'%s' is not a valid IPv6 address"), - string); + _("invalid prefix %s"), + str); return FALSE; } - break; - case 'p': /* IP address + optional prefix */ - { - gs_free char *addr_free = NULL; - const char *addr = string; - const char *str; - - str = strchr(addr, '/'); - if (str) { - addr = nm_strndup_a(200, addr, str - addr, &addr_free); - str++; - if (_nm_utils_ascii_str_to_int64(str, 10, 0, family == AF_INET ? 32 : 128, -1) - < 0) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_FAILED, - _("invalid prefix %s"), - str); - return FALSE; - } - } - if (!nm_utils_ipaddr_is_valid(family, addr)) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_FAILED, - family == AF_INET ? _("'%s' is not a valid IPv4 address") - : _("'%s' is not a valid IPv6 address"), - string); - return FALSE; - } - break; } - case 'T': /* route type. */ - if (!NM_IN_SET(nm_net_aux_rtnl_rtntype_a2n(string), RTN_UNICAST, RTN_LOCAL)) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("%s is not a valid route type"), - string); - return FALSE; - } - break; - default: - break; + if (!nm_utils_ipaddr_is_valid(family, addr)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + family == AF_INET ? _("'%s' is not a valid IPv4 address") + : _("'%s' is not a valid IPv6 address"), + string); + return FALSE; } + break; + } + case 'T': /* route type. */ + string = g_variant_get_string(value, NULL); + if (!NM_IN_SET(nm_net_aux_rtnl_rtntype_a2n(string), RTN_UNICAST, RTN_LOCAL)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("%s is not a valid route type"), + string); + return FALSE; + } + break; + case '\0': + break; + default: + nm_assert_not_reached(); + break; } return TRUE; diff --git a/src/libnm-core-impl/nm-setting-sriov.c b/src/libnm-core-impl/nm-setting-sriov.c index 3875992d0..30dd8f541 100644 --- a/src/libnm-core-impl/nm-setting-sriov.c +++ b/src/libnm-core-impl/nm-setting-sriov.c @@ -350,13 +350,13 @@ nm_sriov_vf_get_attribute(const NMSriovVF *vf, const char *name) const NMVariantAttributeSpec *const _nm_sriov_vf_attribute_spec[] = { NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_SRIOV_VF_ATTRIBUTE_MAC, G_VARIANT_TYPE_STRING, - .str_type = 'm', ), + .type_detail = 'm', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_SRIOV_VF_ATTRIBUTE_SPOOF_CHECK, G_VARIANT_TYPE_BOOLEAN, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_SRIOV_VF_ATTRIBUTE_TRUST, G_VARIANT_TYPE_BOOLEAN, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_SRIOV_VF_ATTRIBUTE_MIN_TX_RATE, G_VARIANT_TYPE_UINT32, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE(NM_SRIOV_VF_ATTRIBUTE_MAX_TX_RATE, G_VARIANT_TYPE_UINT32, ), /* D-Bus only, synthetic attributes */ - NM_VARIANT_ATTRIBUTE_SPEC_DEFINE("vlans", G_VARIANT_TYPE_STRING, .str_type = 'd', ), + NM_VARIANT_ATTRIBUTE_SPEC_DEFINE("vlans", G_VARIANT_TYPE_STRING, .type_detail = 'd', ), NULL, }; @@ -379,6 +379,7 @@ nm_sriov_vf_attribute_validate(const char *name, GVariant *value, gboolean *know { const NMVariantAttributeSpec *const *iter; const NMVariantAttributeSpec *spec = NULL; + const char *string; g_return_val_if_fail(name, FALSE); g_return_val_if_fail(value, FALSE); @@ -391,7 +392,7 @@ nm_sriov_vf_attribute_validate(const char *name, GVariant *value, gboolean *know } } - if (!spec || spec->str_type == 'd') { + if (!spec || spec->type_detail == 'd') { NM_SET_OUT(known, FALSE); g_set_error_literal(error, NM_CONNECTION_ERROR, @@ -411,24 +412,23 @@ nm_sriov_vf_attribute_validate(const char *name, GVariant *value, gboolean *know return FALSE; } - if (g_variant_type_equal(spec->type, G_VARIANT_TYPE_STRING)) { - const char *string; - - switch (spec->str_type) { - case 'm': /* MAC address */ - string = g_variant_get_string(value, NULL); - if (!nm_utils_hwaddr_valid(string, -1)) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_FAILED, - _("'%s' is not a valid MAC address"), - string); - return FALSE; - } - break; - default: - break; + switch (spec->type_detail) { + case 'm': /* MAC address */ + string = g_variant_get_string(value, NULL); + if (!nm_utils_hwaddr_valid(string, -1)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + _("'%s' is not a valid MAC address"), + string); + return FALSE; } + break; + case '\0': + break; + default: + nm_assert_not_reached(); + break; } return TRUE; diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 5a0518f83..54550f273 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3148,7 +3148,13 @@ struct _NMVariantAttributeSpec { bool v6 : 1; bool no_value : 1; bool consumes_rest : 1; - char str_type; + + /* This indicates a non-standard parsing behavior. What this is, + * depends on the actual validation and how to handle it. + * + * Note that the entire NMVariantAttributeSpec is internal API, + * so we can change behavior and adjust it as it fits. */ + char type_detail; }; typedef struct _NMVariantAttributeSpec NMVariantAttributeSpec;