From 54cab39ac99fc6d5ad7378a93bcdb6dc8c340330 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 18 Jun 2021 10:36:17 +0200 Subject: [PATCH] libnm: move gprop_to_dbus_fcn hook to NMSettInfoProperty For each property we have meta data in form of NMSettInfoProperty. Each meta data also has a NMSettInfoProperty.property_type (NMSettInfoPropertType). The property type is supposed to define common behaviors for properties, while the property meta data has individual properties. The idea is that several properties can share the same property-type, and that per-property meta data is part of NMSettInfoProperty. The distinction is not very strong, but note that all remaining uses of NMSettInfoPropertType.gprop_to_dbus_fcn were part of a property type that was only used for one single property. That lack of reusability hints to a wrong use. Move gprop_to_dbus_fcn to the property meta data as a new field NMSettInfoProperty.to_dbus_data. Note that NMSettInfoPropertType.gprop_from_dbus_fcn still suffers from the same problem. But the from-dbus side is not yet addressed. --- src/libnm-core-impl/nm-setting-ip4-config.c | 4 ++-- src/libnm-core-impl/nm-setting-ip6-config.c | 4 ++-- src/libnm-core-impl/nm-setting-private.h | 10 ++++++---- src/libnm-core-impl/nm-setting-serial.c | 4 ++-- .../nm-setting-wireless-security.c | 9 ++++----- src/libnm-core-impl/nm-setting.c | 18 +++++++++--------- src/libnm-core-impl/tests/test-setting.c | 10 +++++++--- src/libnm-core-intern/nm-core-internal.h | 10 ++++++++-- 8 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 205dd4cc08..336e59640d 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -944,8 +944,8 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_DNS), NM_SETT_INFO_PROPERT_TYPE_GPROP(NM_G_VARIANT_TYPE("au"), - .gprop_to_dbus_fcn = ip4_dns_to_dbus, - .gprop_from_dbus_fcn = ip4_dns_from_dbus, )); + .gprop_from_dbus_fcn = ip4_dns_from_dbus, ), + .to_dbus_data.gprop_to_dbus_fcn = ip4_dns_to_dbus); /* ---dbus--- * property: addresses diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index fe171ce884..2bea668150 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -1013,8 +1013,8 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_DNS), NM_SETT_INFO_PROPERT_TYPE_GPROP(NM_G_VARIANT_TYPE("aay"), - .gprop_to_dbus_fcn = ip6_dns_to_dbus, - .gprop_from_dbus_fcn = ip6_dns_from_dbus, )); + .gprop_from_dbus_fcn = ip6_dns_from_dbus, ), + .to_dbus_data.gprop_to_dbus_fcn = ip6_dns_to_dbus); /* ---dbus--- * property: addresses diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 7ce89bd05e..cca1ac37e0 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -362,10 +362,12 @@ _nm_properties_override(GArray *properties_override, const NMSettInfoProperty *p g_array_append_vals(properties_override, prop_info, 1); } -#define _nm_properties_override_gobj(properties_override, p_param_spec, p_property_type) \ - _nm_properties_override( \ - (properties_override), \ - NM_SETT_INFO_PROPERTY(.param_spec = (p_param_spec), .property_type = (p_property_type), )) +#define _nm_properties_override_gobj(properties_override, p_param_spec, p_property_type, ...) \ + _nm_properties_override((properties_override), \ + NM_SETT_INFO_PROPERTY(.name = NULL, \ + .param_spec = (p_param_spec), \ + .property_type = (p_property_type), \ + __VA_ARGS__)) #define _nm_properties_override_dbus(properties_override, p_name, p_property_type) \ _nm_properties_override( \ diff --git a/src/libnm-core-impl/nm-setting-serial.c b/src/libnm-core-impl/nm-setting-serial.c index 22244c7746..c87de166ac 100644 --- a/src/libnm-core-impl/nm-setting-serial.c +++ b/src/libnm-core-impl/nm-setting-serial.c @@ -312,8 +312,8 @@ nm_setting_serial_class_init(NMSettingSerialClass *klass) properties_override, obj_properties[PROP_PARITY], NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_BYTE, - .gprop_to_dbus_fcn = parity_to_dbus, - .gprop_from_dbus_fcn = parity_from_dbus, )); + .gprop_from_dbus_fcn = parity_from_dbus, ), + .to_dbus_data.gprop_to_dbus_fcn = parity_to_dbus, ); /** * NMSettingSerial:stopbits: diff --git a/src/libnm-core-impl/nm-setting-wireless-security.c b/src/libnm-core-impl/nm-setting-wireless-security.c index 39f3a95fc9..9e3c8d819f 100644 --- a/src/libnm-core-impl/nm-setting-wireless-security.c +++ b/src/libnm-core-impl/nm-setting-wireless-security.c @@ -1924,11 +1924,10 @@ nm_setting_wireless_security_class_init(NMSettingWirelessSecurityClass *klass) NM_TYPE_WEP_KEY_TYPE, NM_WEP_KEY_TYPE_UNKNOWN, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); - _nm_properties_override_gobj( - properties_override, - obj_properties[PROP_WEP_KEY_TYPE], - NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_UINT32, - .gprop_to_dbus_fcn = wep_key_type_to_dbus, )); + _nm_properties_override_gobj(properties_override, + obj_properties[PROP_WEP_KEY_TYPE], + &nm_sett_info_propert_type_plain_u, + .to_dbus_data.gprop_to_dbus_fcn = wep_key_type_to_dbus, ); /** * NMSettingWirelessSecurity:wps-method: diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index 74b43dfeb7..5fbb66dbac 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -171,15 +171,11 @@ _nm_properties_override_assert(const NMSettInfoProperty *prop_info) /* we always require a dbus_type. */ nm_assert(property_type->dbus_type); - if (!property_type->to_dbus_fcn) - nm_assert(!property_type->gprop_to_dbus_fcn); - - /* {to,from}_dbus_fcn and gprop_{to,from}_dbus_fcn cannot both be set. */ + /* from_dbus_fcn and gprop_from_dbus_fcn cannot both be set. */ nm_assert(!property_type->from_dbus_fcn || !property_type->gprop_from_dbus_fcn); if (!prop_info->param_spec) { - /* if we don't have a param_spec, we cannot have gprop_{to,from}_dbus_fcn. */ - nm_assert(!property_type->gprop_to_dbus_fcn); + /* if we don't have a param_spec, we cannot have gprop_from_dbus_fcn. */ nm_assert(!property_type->gprop_from_dbus_fcn); } } @@ -550,6 +546,10 @@ _nm_setting_property_to_dbus_fcn_gprop(const NMSettInfoSetting * s GArray *tmp_array; nm_assert(property->param_spec); + nm_assert(property->property_type->to_dbus_fcn == _nm_setting_property_to_dbus_fcn_gprop); + nm_assert(property->property_type->typdata_to_dbus.gprop_type + == NM_SETTING_PROPERTY_TO_DBUS_FCN_GPROP_TYPE_DEFAULT + || !property->to_dbus_data.gprop_to_dbus_fcn); g_value_init(&prop_value, property->param_spec->value_type); @@ -560,8 +560,8 @@ _nm_setting_property_to_dbus_fcn_gprop(const NMSettInfoSetting * s switch (property->property_type->typdata_to_dbus.gprop_type) { case NM_SETTING_PROPERTY_TO_DBUS_FCN_GPROP_TYPE_DEFAULT: - if (property->property_type->gprop_to_dbus_fcn) - return property->property_type->gprop_to_dbus_fcn(&prop_value); + if (property->to_dbus_data.gprop_to_dbus_fcn) + return property->to_dbus_data.gprop_to_dbus_fcn(&prop_value); return g_dbus_gvalue_to_gvariant(&prop_value, property->property_type->dbus_type); case NM_SETTING_PROPERTY_TO_DBUS_FCN_GPROP_TYPE_BYTES: @@ -603,7 +603,7 @@ property_to_dbus(const NMSettInfoSetting * sett_info, if (!property->property_type->to_dbus_fcn) { nm_assert(!property->param_spec); - nm_assert(!property->property_type->gprop_to_dbus_fcn); + nm_assert(!property->to_dbus_data.none); return NULL; } diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index 567eb81df4..6a342bcca8 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -4372,7 +4372,7 @@ test_setting_metadata(void) /* it's allowed to have no to_dbus_fcn(), to ignore a property. But such * properties must not have a param_spec and no gprop_to_dbus_fcn. */ g_assert(!sip->param_spec); - g_assert(!sip->property_type->gprop_to_dbus_fcn); + g_assert(!sip->to_dbus_data.none); } else if (sip->property_type->to_dbus_fcn == _nm_setting_property_to_dbus_fcn_gprop) { g_assert(sip->param_spec); switch (sip->property_type->typdata_to_dbus.gprop_type) { @@ -4399,6 +4399,9 @@ test_setting_metadata(void) } g_assert_not_reached(); check_done:; + if (sip->property_type->typdata_to_dbus.gprop_type + != NM_SETTING_PROPERTY_TO_DBUS_FCN_GPROP_TYPE_DEFAULT) + g_assert(!sip->to_dbus_data.gprop_to_dbus_fcn); } g_assert(!sip->property_type->from_dbus_fcn @@ -4518,7 +4521,6 @@ check_done:; || pt->to_dbus_fcn != pt_2->to_dbus_fcn || pt->from_dbus_fcn != pt_2->from_dbus_fcn || pt->missing_from_dbus_fcn != pt_2->missing_from_dbus_fcn - || pt->gprop_to_dbus_fcn != pt_2->gprop_to_dbus_fcn || pt->gprop_from_dbus_fcn != pt_2->gprop_from_dbus_fcn || memcmp(&pt->typdata_to_dbus, &pt_2->typdata_to_dbus, @@ -4545,7 +4547,9 @@ check_done:; /* the property-types with same content should all be shared. Here we have two that * are the same content, but different instances. Bug. */ - g_error("The identical property type for D-Bus type \"%s\" is used by: %s and %s", + g_error("The identical property type for D-Bus type \"%s\" is used by: %s and %s. " + "If a NMSettInfoPropertType is identical, it should be shared by creating " + "a common instance of the property type", (const char *) pt->dbus_type, _PROP_IDX_OWNER(h_property_types, pt), _PROP_IDX_OWNER(h_property_types, pt_2)); diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 7678bcef80..657c66d683 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -688,9 +688,8 @@ typedef struct { NMSettInfoPropFromDBusFcn from_dbus_fcn; NMSettInfoPropMissingFromDBusFcn missing_from_dbus_fcn; - /* Simpler variants of @to_dbus_fcn/@from_dbus_fcn that operate solely + /* Simpler variants of @from_dbus_fcn that operate solely * on the GValue value of the GObject property. */ - NMSettInfoPropGPropToDBusFcn gprop_to_dbus_fcn; NMSettInfoPropGPropFromDBusFcn gprop_from_dbus_fcn; struct { @@ -707,6 +706,13 @@ struct _NMSettInfoProperty { GParamSpec *param_spec; const NMSettInfoPropertType *property_type; + + struct { + union { + gpointer none; + NMSettInfoPropGPropToDBusFcn gprop_to_dbus_fcn; + }; + } to_dbus_data; }; typedef struct {