From c161439b73c4bc7fcb91e45d3bdef1471b03eab0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 18 Jun 2021 08:59:12 +0200 Subject: libnm: let all property types implement to_dbus_fcn() handler If a property can be converted to D-Bus, then always set the to_dbus_fcn() handler. The only caller of to_dbus_fcn() is property_to_dbus(), so this means that property_to_dbus() has no more default implementation and always delegates to to_dbus_fcn(). The code is easier to understand if all properties implement to_dbus_fcn() the same way. Also, there is supposed to be a split between NMSettInfoProperty (info about the property) and NMSettInfoPropertType (the type). The idea is that each property (obviously) requires its distinct NMSettInfoProperty, but they can share a common type implementation. With NMSettInfoPropertType.gprop_to_dbus_fcn that is often violated because many properties that implement NMSettInfoPropertType.gprop_to_dbus_fcn require a special type implementation. As such, gprop_to_dbus_fcn should be part of the property info and not the property type. The first step towards that is unifying all properties to use to_dbus_fcn(). --- src/libnm-core-impl/nm-setting-connection.c | 6 +- src/libnm-core-impl/nm-setting-dcb.c | 1 + src/libnm-core-impl/nm-setting-ip-config.c | 8 +- src/libnm-core-impl/nm-setting-ip4-config.c | 6 +- src/libnm-core-impl/nm-setting-ip6-config.c | 6 +- src/libnm-core-impl/nm-setting-private.h | 12 +++ src/libnm-core-impl/nm-setting-serial.c | 6 +- src/libnm-core-impl/nm-setting-wireless-security.c | 4 +- src/libnm-core-impl/nm-setting.c | 107 ++++++++++++--------- src/libnm-core-impl/nm-utils.c | 2 + src/libnm-core-impl/tests/test-setting.c | 8 +- 11 files changed, 100 insertions(+), 66 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index ebed3ef9bb..fbe2eee920 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -1995,9 +1995,9 @@ nm_setting_connection_class_init(NMSettingConnectionClass *klass) _nm_properties_override_gobj( properties_override, obj_properties[PROP_INTERFACE_NAME], - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_STRING, - .missing_from_dbus_fcn = - nm_setting_connection_no_interface_name, )); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_STRING, + .missing_from_dbus_fcn = + nm_setting_connection_no_interface_name, )); /** * NMSettingConnection:type: diff --git a/src/libnm-core-impl/nm-setting-dcb.c b/src/libnm-core-impl/nm-setting-dcb.c index 89df5835e8..228a89941c 100644 --- a/src/libnm-core-impl/nm-setting-dcb.c +++ b/src/libnm-core-impl/nm-setting-dcb.c @@ -764,6 +764,7 @@ _nm_setting_dcb_uint_array_from_dbus(GVariant *dbus_value, GValue *prop_value) static const NMSettInfoPropertType nm_sett_info_propert_type_dcb_au = { .dbus_type = NM_G_VARIANT_TYPE("au"), + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, .gprop_to_dbus_fcn = _nm_setting_dcb_uint_array_to_dbus, .gprop_from_dbus_fcn = _nm_setting_dcb_uint_array_from_dbus, }; diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index aa1bf52bff..574dfd2cbb 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -5766,10 +5766,10 @@ _nm_sett_info_property_override_create_array_ip_config(void) { GArray *properties_override = _nm_sett_info_property_override_create_array(); - _nm_properties_override_gobj(properties_override, - obj_properties[PROP_GATEWAY], - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_STRING, - .from_dbus_fcn = ip_gateway_set, )); + _nm_properties_override_gobj( + properties_override, + obj_properties[PROP_GATEWAY], + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_STRING, .from_dbus_fcn = ip_gateway_set, )); /* ---dbus--- * property: routing-rules diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 003e8dc131..d5eee5fe42 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -943,9 +943,9 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) _nm_properties_override_gobj( properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_DNS), - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = NM_G_VARIANT_TYPE("au"), - .gprop_to_dbus_fcn = ip4_dns_to_dbus, - .gprop_from_dbus_fcn = ip4_dns_from_dbus, )); + 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, )); /* ---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 d2a016fe25..bd7bd7aca0 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -1012,9 +1012,9 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) _nm_properties_override_gobj( properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_DNS), - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = NM_G_VARIANT_TYPE("aay"), - .gprop_to_dbus_fcn = ip6_dns_to_dbus, - .gprop_from_dbus_fcn = ip6_dns_from_dbus, )); + 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, )); /* ---dbus--- * property: addresses diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index b711b8be8f..653b7a3f2e 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -265,6 +265,13 @@ gboolean _nm_setting_aggregate(NMSetting *setting, NMConnectionAggregateType typ gboolean _nm_setting_slave_type_is_valid(const char *slave_type, const char **out_port_type); +GVariant *_nm_setting_property_to_dbus_fcn_gprop(const NMSettInfoSetting * sett_info, + guint property_idx, + NMConnection * connection, + NMSetting * setting, + NMConnectionSerializationFlags flags, + const NMConnectionSerializationOptions *options); + GVariant *_nm_setting_to_dbus(NMSetting * setting, NMConnection * connection, NMConnectionSerializationFlags flags, @@ -326,6 +333,11 @@ _nm_setting_class_commit(NMSettingClass *setting_class, NMMetaSettingType meta_t &_g; \ }) +#define NM_SETT_INFO_PROPERT_TYPE_GPROP(_dbus_type, ...) \ + NM_SETT_INFO_PROPERT_TYPE(.dbus_type = _dbus_type, \ + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, \ + __VA_ARGS__) + #define NM_SETT_INFO_PROPERTY(...) (&((const NMSettInfoProperty){__VA_ARGS__})) gboolean _nm_properties_override_assert(const NMSettInfoProperty *prop_info); diff --git a/src/libnm-core-impl/nm-setting-serial.c b/src/libnm-core-impl/nm-setting-serial.c index 5a3802a3e7..22244c7746 100644 --- a/src/libnm-core-impl/nm-setting-serial.c +++ b/src/libnm-core-impl/nm-setting-serial.c @@ -311,9 +311,9 @@ nm_setting_serial_class_init(NMSettingSerialClass *klass) _nm_properties_override_gobj( properties_override, obj_properties[PROP_PARITY], - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_BYTE, - .gprop_to_dbus_fcn = parity_to_dbus, - .gprop_from_dbus_fcn = parity_from_dbus, )); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_BYTE, + .gprop_to_dbus_fcn = parity_to_dbus, + .gprop_from_dbus_fcn = parity_from_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 df618e36d9..39f3a95fc9 100644 --- a/src/libnm-core-impl/nm-setting-wireless-security.c +++ b/src/libnm-core-impl/nm-setting-wireless-security.c @@ -1927,8 +1927,8 @@ nm_setting_wireless_security_class_init(NMSettingWirelessSecurityClass *klass) _nm_properties_override_gobj( properties_override, obj_properties[PROP_WEP_KEY_TYPE], - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_UINT32, - .gprop_to_dbus_fcn = wep_key_type_to_dbus, )); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_UINT32, + .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 31c5030ce0..81ec3e8621 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -190,8 +190,10 @@ _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. */ - nm_assert(!property_type->to_dbus_fcn || !property_type->gprop_to_dbus_fcn); nm_assert(!property_type->from_dbus_fcn || !property_type->gprop_from_dbus_fcn); if (!prop_info->param_spec) { @@ -378,35 +380,35 @@ _nm_setting_class_commit_full(NMSettingClass * setting_class, vtype = p->param_spec->value_type; if (vtype == G_TYPE_BOOLEAN) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_BOOLEAN); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_BOOLEAN); else if (vtype == G_TYPE_UCHAR) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_BYTE); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_BYTE); else if (vtype == G_TYPE_INT) p->property_type = &nm_sett_info_propert_type_plain_i; else if (vtype == G_TYPE_UINT) p->property_type = &nm_sett_info_propert_type_plain_u; else if (vtype == G_TYPE_INT64) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_INT64); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_INT64); else if (vtype == G_TYPE_UINT64) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_UINT64); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_UINT64); else if (vtype == G_TYPE_STRING) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_STRING); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_STRING); else if (vtype == G_TYPE_DOUBLE) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_DOUBLE); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_DOUBLE); else if (vtype == G_TYPE_STRV) - p->property_type = NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_STRING_ARRAY); + p->property_type = NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_STRING_ARRAY); else if (vtype == G_TYPE_BYTES) { p->property_type = - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_BYTESTRING, - .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_bytes); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_BYTESTRING, + .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_bytes); } else if (g_type_is_a(vtype, G_TYPE_ENUM)) { p->property_type = - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_INT32, - .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_enum); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_INT32, + .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_enum); } else if (g_type_is_a(vtype, G_TYPE_FLAGS)) { p->property_type = - NM_SETT_INFO_PROPERT_TYPE(.dbus_type = G_VARIANT_TYPE_UINT32, - .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_flags); + NM_SETT_INFO_PROPERT_TYPE_GPROP(G_VARIANT_TYPE_UINT32, + .gprop_to_dbus_fcn = _gprop_to_dbus_fcn_flags); } else nm_assert_not_reached(); @@ -552,6 +554,34 @@ _nm_setting_use_legacy_property(NMSetting * setting, /*****************************************************************************/ +GVariant * +_nm_setting_property_to_dbus_fcn_gprop(const NMSettInfoSetting * sett_info, + guint property_idx, + NMConnection * connection, + NMSetting * setting, + NMConnectionSerializationFlags flags, + const NMConnectionSerializationOptions *options) +{ + const NMSettInfoProperty *const property = &sett_info->property_infos[property_idx]; + nm_auto_unset_gvalue GValue prop_value = { + 0, + }; + + nm_assert(property->param_spec); + + g_value_init(&prop_value, property->param_spec->value_type); + + g_object_get_property(G_OBJECT(setting), property->param_spec->name, &prop_value); + + if (g_param_value_defaults(property->param_spec, &prop_value)) + return NULL; + + if (property->property_type->gprop_to_dbus_fcn) + return property->property_type->gprop_to_dbus_fcn(&prop_value); + + return g_dbus_gvalue_to_gvariant(&prop_value, property->property_type->dbus_type); +} + static GVariant * property_to_dbus(const NMSettInfoSetting * sett_info, guint property_idx, @@ -566,12 +596,15 @@ property_to_dbus(const NMSettInfoSetting * sett_info, nm_assert(property->property_type->dbus_type); - if (!property->param_spec) { - if (!property->property_type->to_dbus_fcn) - return NULL; - } else if (!ignore_flags - && !NM_FLAGS_HAS(property->param_spec->flags, - NM_SETTING_PARAM_TO_DBUS_IGNORE_FLAGS)) { + if (!property->property_type->to_dbus_fcn) { + nm_assert(!property->param_spec); + nm_assert(!property->property_type->gprop_to_dbus_fcn); + return NULL; + } + + if (property->param_spec + && (!ignore_flags + && !NM_FLAGS_HAS(property->param_spec->flags, NM_SETTING_PARAM_TO_DBUS_IGNORE_FLAGS))) { if (!NM_FLAGS_HAS(property->param_spec->flags, G_PARAM_WRITABLE)) return NULL; @@ -598,32 +631,10 @@ property_to_dbus(const NMSettInfoSetting * sett_info, } } - if (property->property_type->to_dbus_fcn) { - variant = property->property_type - ->to_dbus_fcn(sett_info, property_idx, connection, setting, flags, options); - nm_g_variant_take_ref(variant); - } else { - nm_auto_unset_gvalue GValue prop_value = { - 0, - }; - - nm_assert(property->param_spec); - - g_value_init(&prop_value, property->param_spec->value_type); - - g_object_get_property(G_OBJECT(setting), property->param_spec->name, &prop_value); + variant = property->property_type + ->to_dbus_fcn(sett_info, property_idx, connection, setting, flags, options); + nm_g_variant_take_ref(variant); - if (g_param_value_defaults(property->param_spec, &prop_value)) - return NULL; - - if (property->property_type->gprop_to_dbus_fcn) { - variant = property->property_type->gprop_to_dbus_fcn(&prop_value); - nm_g_variant_take_ref(variant); - } else - variant = g_dbus_gvalue_to_gvariant(&prop_value, property->property_type->dbus_type); - } - - nm_assert(!variant || !g_variant_is_floating(variant)); nm_assert(!variant || g_variant_is_of_type(variant, property->property_type->dbus_type)); return variant; @@ -2333,11 +2344,13 @@ const NMSettInfoPropertType nm_sett_info_propert_type_deprecated_ignore_u = { }; const NMSettInfoPropertType nm_sett_info_propert_type_plain_i = { - .dbus_type = G_VARIANT_TYPE_INT32, + .dbus_type = G_VARIANT_TYPE_INT32, + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, }; const NMSettInfoPropertType nm_sett_info_propert_type_plain_u = { - .dbus_type = G_VARIANT_TYPE_UINT32, + .dbus_type = G_VARIANT_TYPE_UINT32, + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, }; /*****************************************************************************/ diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index 391dd1a15e..f42c81d9d2 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -781,6 +781,7 @@ _nm_utils_strdict_from_dbus(GVariant *dbus_value, GValue *prop_value) const NMSettInfoPropertType nm_sett_info_propert_type_strdict = { .dbus_type = NM_G_VARIANT_TYPE("a{ss}"), + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, .gprop_to_dbus_fcn = _nm_utils_strdict_to_dbus, .gprop_from_dbus_fcn = _nm_utils_strdict_from_dbus, }; @@ -4156,6 +4157,7 @@ _nm_utils_hwaddr_from_dbus(GVariant *dbus_value, GValue *prop_value) const NMSettInfoPropertType nm_sett_info_propert_type_mac_address = { .dbus_type = G_VARIANT_TYPE_BYTESTRING, + .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_gprop, .gprop_to_dbus_fcn = _nm_utils_hwaddr_to_dbus, .gprop_from_dbus_fcn = _nm_utils_hwaddr_from_dbus, }; diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index 50821bb16c..835dc80d01 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -4368,7 +4368,13 @@ test_setting_metadata(void) g_assert(sip->property_type->dbus_type); g_assert(g_variant_type_string_is_valid((const char *) sip->property_type->dbus_type)); - g_assert(!sip->property_type->to_dbus_fcn || !sip->property_type->gprop_to_dbus_fcn); + if (!sip->property_type->to_dbus_fcn) { + /* 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->property_type->from_dbus_fcn || !sip->property_type->gprop_from_dbus_fcn); -- cgit v1.2.3