From 27621dde45586b8beba773966ceac4f1c61d7e75 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 28 Jun 2021 23:51:42 +0200 Subject: [PATCH] libnm: add generic cleanup function to finalize NMSetting If all settings would be strictly be implemented as "direct" properties, we could call this from NMSetting.finalize() and be done with it. As it is, for now we cannot, so it's still cumbersome. --- src/libnm-core-impl/nm-setting-6lowpan.c | 15 +--- src/libnm-core-impl/nm-setting-private.h | 99 ++++++++++++++---------- src/libnm-core-impl/nm-setting.c | 47 ++++++++++- 3 files changed, 104 insertions(+), 57 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-6lowpan.c b/src/libnm-core-impl/nm-setting-6lowpan.c index c7f1aca66e..fe7bf1c566 100644 --- a/src/libnm-core-impl/nm-setting-6lowpan.c +++ b/src/libnm-core-impl/nm-setting-6lowpan.c @@ -147,17 +147,6 @@ nm_setting_6lowpan_new(void) return g_object_new(NM_TYPE_SETTING_6LOWPAN, NULL); } -static void -finalize(GObject *object) -{ - NMSetting6Lowpan * setting = NM_SETTING_6LOWPAN(object); - NMSetting6LowpanPrivate *priv = NM_SETTING_6LOWPAN_GET_PRIVATE(setting); - - g_free(priv->parent); - - G_OBJECT_CLASS(nm_setting_6lowpan_parent_class)->finalize(object); -} - static void nm_setting_6lowpan_class_init(NMSetting6LowpanClass *klass) { @@ -169,9 +158,9 @@ nm_setting_6lowpan_class_init(NMSetting6LowpanClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - object_class->finalize = finalize; - setting_class->verify = verify; + setting_class->verify = verify; + setting_class->finalize_direct = TRUE; /** * NMSetting6Lowpan:parent: diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index e35543e170..047dccb283 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -115,7 +115,20 @@ struct _NMSettingClass { guint /* NMSettingParseFlags */ parse_flags, GError ** error); - gpointer padding[1]; + union { + gpointer padding[1]; + struct { + /* Whether NMSetting.finalize() calls _nm_setting_property_finalize_direct(). Subclasses + * need to be aware of that, and currently this is opt-in. + * + * The only reason because subclasses need to be aware of this, is that they + * otherwise might clear the properties already and leave dangling pointers. + * + * Eventually all setting classes should stop touching their direct properties + * during finalize, and always let NMSetting.finalize() handle them. */ + bool finalize_direct : 1; + }; + }; const struct _NMMetaSettingInfo *setting_info; }; @@ -483,47 +496,47 @@ _nm_properties_override(GArray *properties_override, const NMSettInfoProperty *p /*****************************************************************************/ -#define _nm_setting_property_define_direct_string_full(properties_override, \ - obj_properties, \ - prop_name, \ - prop_id, \ - param_flags, \ - property_type, \ - private_struct_type, \ - private_struct_field, \ - ...) \ - G_STMT_START \ - { \ - GParamSpec * _param_spec; \ - const NMSettInfoPropertType *_property_type = (property_type); \ - \ - G_STATIC_ASSERT(!NM_FLAGS_ANY((param_flags), \ - ~(NM_SETTING_PARAM_SECRET | NM_SETTING_PARAM_FUZZY_IGNORE \ - | NM_SETTING_PARAM_INFERRABLE \ - | NM_SETTING_PARAM_REAPPLY_IMMEDIATELY))); \ - \ - nm_assert(_property_type); \ - nm_assert(g_variant_type_equal(_property_type->dbus_type, "s")); \ - nm_assert(_property_type->direct_type == NM_VALUE_TYPE_STRING); \ - nm_assert(_property_type->to_dbus_fcn == _nm_setting_property_to_dbus_fcn_direct); \ - \ - _param_spec = \ - g_param_spec_string("" prop_name "", \ - "", \ - "", \ - NULL, \ - G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS | (param_flags)); \ - \ - (obj_properties)[(prop_id)] = _param_spec; \ - \ - _nm_properties_override_gobj( \ - (properties_override), \ - _param_spec, \ - _property_type, \ - .direct_offset = \ - NM_STRUCT_OFFSET_ENSURE_TYPE(char *, private_struct_type, private_struct_field), \ - __VA_ARGS__); \ - } \ +#define _nm_setting_property_define_direct_string_full(properties_override, \ + obj_properties, \ + prop_name, \ + prop_id, \ + param_flags, \ + property_type, \ + private_struct_type, \ + private_struct_field, \ + ... /* extra NMSettInfoProperty fields */) \ + G_STMT_START \ + { \ + GParamSpec * _param_spec; \ + const NMSettInfoPropertType *_property_type = (property_type); \ + \ + G_STATIC_ASSERT(!NM_FLAGS_ANY((param_flags), \ + ~(NM_SETTING_PARAM_SECRET | NM_SETTING_PARAM_FUZZY_IGNORE \ + | NM_SETTING_PARAM_INFERRABLE \ + | NM_SETTING_PARAM_REAPPLY_IMMEDIATELY))); \ + \ + nm_assert(_property_type); \ + nm_assert(g_variant_type_equal(_property_type->dbus_type, "s")); \ + nm_assert(_property_type->direct_type == NM_VALUE_TYPE_STRING); \ + nm_assert(_property_type->to_dbus_fcn == _nm_setting_property_to_dbus_fcn_direct); \ + \ + _param_spec = \ + g_param_spec_string("" prop_name "", \ + "", \ + "", \ + NULL, \ + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS | (param_flags)); \ + \ + (obj_properties)[(prop_id)] = _param_spec; \ + \ + _nm_properties_override_gobj( \ + (properties_override), \ + _param_spec, \ + _property_type, \ + .direct_offset = \ + NM_STRUCT_OFFSET_ENSURE_TYPE(char *, private_struct_type, private_struct_field), \ + __VA_ARGS__); \ + } \ G_STMT_END #define _nm_setting_property_define_direct_string(properties_override, \ @@ -533,7 +546,7 @@ _nm_properties_override(GArray *properties_override, const NMSettInfoProperty *p param_flags, \ private_struct_type, \ private_struct_field, \ - ...) \ + ... /* extra NMSettInfoProperty fields */) \ _nm_setting_property_define_direct_string_full((properties_override), \ (obj_properties), \ prop_name, \ diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index faad3bcae6..5ae7296263 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -730,6 +730,46 @@ out_fail: /*****************************************************************************/ +static void +_finalize_direct(NMSetting *setting) +{ + const NMSettInfoSetting *sett_info; + guint i; + + sett_info = _nm_setting_class_get_sett_info(NM_SETTING_GET_CLASS(setting)); + nm_assert(sett_info); + + for (i = 0; i < sett_info->property_infos_len; i++) { + const NMSettInfoProperty *property_info = &sett_info->property_infos[i]; + + /* We only: + * + * - reset fields where there is something to free. E.g. boolean + * properties are not reset to their default. + * - clear/free properties, without emitting g_object_notify_by_pspec(), + * because this is called only during finalization. */ + + switch (property_info->property_type->direct_type) { + case NM_VALUE_TYPE_NONE: + case NM_VALUE_TYPE_BOOL: + break; + case NM_VALUE_TYPE_STRING: + { + char **p_val = + _nm_setting_get_private(setting, sett_info, property_info->direct_offset); + + nm_clear_g_free(p_val); + break; + } + default: + nm_assert_not_reached(); + break; + } + } +} + +/*****************************************************************************/ + GVariant * _nm_setting_property_to_dbus_fcn_direct(const NMSettInfoSetting * sett_info, guint property_idx, @@ -3052,7 +3092,9 @@ nm_setting_init(NMSetting *setting) static void finalize(GObject *object) { - NMSettingPrivate *priv = NM_SETTING_GET_PRIVATE(object); + NMSetting * self = NM_SETTING(object); + NMSettingPrivate *priv = NM_SETTING_GET_PRIVATE(self); + NMSettingClass * klass = NM_SETTING_GET_CLASS(self); if (priv->gendata) { g_free(priv->gendata->names); @@ -3062,6 +3104,9 @@ finalize(GObject *object) } G_OBJECT_CLASS(nm_setting_parent_class)->finalize(object); + + if (klass->finalize_direct) + _finalize_direct(self); } static void