From 989a6911ba0b269aebeaa82e4a50a8d9efade6f5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 23 Oct 2021 20:58:27 +0200 Subject: [PATCH] libnm: always finalize direct properties in NMSetting base class Certain properties need to release memory when destroying the NMSetting. For "direct" properties, we have all the information we need to do that generically in the NMSetting base class. In practice, this only concerns string properties. See _finalize_direct() in "nm-setting.c". However, if the NMSetting base class takes care of freeing the strings, then the subclasses must not also unref the variable (to avoid double free). Previously, subclasses had to opt-in for the base class to indicate that they are fine with that. Now, let the base class always handle it. We only need to make sure that classes that implement direct string properties don't also try to free the values during destruction. --- src/libnm-core-impl/nm-setting-6lowpan.c | 3 +-- src/libnm-core-impl/nm-setting-adsl.c | 14 -------------- src/libnm-core-impl/nm-setting-bluetooth.c | 3 +-- src/libnm-core-impl/nm-setting-bond-port.c | 3 +-- src/libnm-core-impl/nm-setting-bridge.c | 2 -- src/libnm-core-impl/nm-setting-connection.c | 9 --------- src/libnm-core-impl/nm-setting-infiniband.c | 1 - src/libnm-core-impl/nm-setting-ip-config.c | 5 ----- src/libnm-core-impl/nm-setting-ip-tunnel.c | 16 ---------------- src/libnm-core-impl/nm-setting-ip4-config.c | 13 ------------- src/libnm-core-impl/nm-setting-ip6-config.c | 13 ------------- src/libnm-core-impl/nm-setting-olpc-mesh.c | 1 - src/libnm-core-impl/nm-setting-ovs-port.c | 3 +-- src/libnm-core-impl/nm-setting-ppp.c | 3 +-- src/libnm-core-impl/nm-setting-private.h | 15 --------------- src/libnm-core-impl/nm-setting-tun.c | 3 +-- src/libnm-core-impl/nm-setting-vrf.c | 3 +-- src/libnm-core-impl/nm-setting-wimax.c | 1 - src/libnm-core-impl/nm-setting-wired.c | 8 -------- src/libnm-core-impl/nm-setting-wireless.c | 2 -- src/libnm-core-impl/nm-setting-wpan.c | 3 +-- src/libnm-core-impl/nm-setting.c | 8 +++----- 22 files changed, 11 insertions(+), 121 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-6lowpan.c b/src/libnm-core-impl/nm-setting-6lowpan.c index fe7bf1c566..606fa8243c 100644 --- a/src/libnm-core-impl/nm-setting-6lowpan.c +++ b/src/libnm-core-impl/nm-setting-6lowpan.c @@ -159,8 +159,7 @@ 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; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSetting6Lowpan:parent: diff --git a/src/libnm-core-impl/nm-setting-adsl.c b/src/libnm-core-impl/nm-setting-adsl.c index 5f1888d0e4..b18dba827a 100644 --- a/src/libnm-core-impl/nm-setting-adsl.c +++ b/src/libnm-core-impl/nm-setting-adsl.c @@ -335,19 +335,6 @@ nm_setting_adsl_new(void) return g_object_new(NM_TYPE_SETTING_ADSL, NULL); } -static void -finalize(GObject *object) -{ - NMSettingAdslPrivate *priv = NM_SETTING_ADSL_GET_PRIVATE(object); - - g_free(priv->username); - g_free(priv->password); - g_free(priv->protocol); - g_free(priv->encapsulation); - - G_OBJECT_CLASS(nm_setting_adsl_parent_class)->finalize(object); -} - static void nm_setting_adsl_class_init(NMSettingAdslClass *klass) { @@ -359,7 +346,6 @@ nm_setting_adsl_class_init(NMSettingAdslClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; - object_class->finalize = finalize; setting_class->verify = verify; setting_class->verify_secrets = verify_secrets; diff --git a/src/libnm-core-impl/nm-setting-bluetooth.c b/src/libnm-core-impl/nm-setting-bluetooth.c index f0b4a3914e..53e4912f79 100644 --- a/src/libnm-core-impl/nm-setting-bluetooth.c +++ b/src/libnm-core-impl/nm-setting-bluetooth.c @@ -259,8 +259,7 @@ nm_setting_bluetooth_class_init(NMSettingBluetoothClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingBluetooth:bdaddr: diff --git a/src/libnm-core-impl/nm-setting-bond-port.c b/src/libnm-core-impl/nm-setting-bond-port.c index 5eb1bfd07f..4538e8b9f5 100644 --- a/src/libnm-core-impl/nm-setting-bond-port.c +++ b/src/libnm-core-impl/nm-setting-bond-port.c @@ -134,8 +134,7 @@ nm_setting_bond_port_class_init(NMSettingBondPortClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingBondPort:queue-id: diff --git a/src/libnm-core-impl/nm-setting-bridge.c b/src/libnm-core-impl/nm-setting-bridge.c index 0c5bbed138..7c86d30af5 100644 --- a/src/libnm-core-impl/nm-setting-bridge.c +++ b/src/libnm-core-impl/nm-setting-bridge.c @@ -1569,9 +1569,7 @@ finalize(GObject *object) { NMSettingBridgePrivate *priv = NM_SETTING_BRIDGE_GET_PRIVATE(object); - g_free(priv->mac_address); g_free(priv->multicast_router); - g_free(priv->group_address); g_free(priv->vlan_protocol); g_ptr_array_unref(priv->vlans); diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 89555540ad..7e4764f03f 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -1860,15 +1860,6 @@ finalize(GObject *object) { NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE(object); - g_free(priv->id); - g_free(priv->uuid); - g_free(priv->stable_id); - g_free(priv->interface_name); - g_free(priv->type); - g_free(priv->zone); - g_free(priv->master); - g_free(priv->slave_type); - g_free(priv->mud_url); nm_clear_pointer(&priv->permissions, g_array_unref); nm_clear_pointer(&priv->secondaries, g_array_unref); diff --git a/src/libnm-core-impl/nm-setting-infiniband.c b/src/libnm-core-impl/nm-setting-infiniband.c index 117d89fced..adbf3636a6 100644 --- a/src/libnm-core-impl/nm-setting-infiniband.c +++ b/src/libnm-core-impl/nm-setting-infiniband.c @@ -379,7 +379,6 @@ finalize(GObject *object) NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE(object); g_free(priv->transport_mode); - g_free(priv->mac_address); g_free(priv->parent); g_free(priv->virtual_iface_name); diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 182ab0188c..33779ed952 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -6082,11 +6082,6 @@ finalize(GObject *object) NMSettingIPConfig * self = NM_SETTING_IP_CONFIG(object); NMSettingIPConfigPrivate *priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(self); - g_free(priv->method); - g_free(priv->gateway); - g_free(priv->dhcp_hostname); - g_free(priv->dhcp_iaid); - g_ptr_array_unref(priv->dns); g_ptr_array_unref(priv->dns_search); if (priv->dns_options) diff --git a/src/libnm-core-impl/nm-setting-ip-tunnel.c b/src/libnm-core-impl/nm-setting-ip-tunnel.c index f75ca1d5b7..38f9047dca 100644 --- a/src/libnm-core-impl/nm-setting-ip-tunnel.c +++ b/src/libnm-core-impl/nm-setting-ip-tunnel.c @@ -632,21 +632,6 @@ nm_setting_ip_tunnel_new(void) return g_object_new(NM_TYPE_SETTING_IP_TUNNEL, NULL); } -static void -finalize(GObject *object) -{ - NMSettingIPTunnel * setting = NM_SETTING_IP_TUNNEL(object); - NMSettingIPTunnelPrivate *priv = NM_SETTING_IP_TUNNEL_GET_PRIVATE(setting); - - g_free(priv->parent); - g_free(priv->local); - g_free(priv->remote); - g_free(priv->input_key); - g_free(priv->output_key); - - G_OBJECT_CLASS(nm_setting_ip_tunnel_parent_class)->finalize(object); -} - static void nm_setting_ip_tunnel_class_init(NMSettingIPTunnelClass *klass) { @@ -658,7 +643,6 @@ nm_setting_ip_tunnel_class_init(NMSettingIPTunnelClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; - object_class->finalize = finalize; setting_class->verify = verify; diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 6f13acbfb5..a7b3d68bd3 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -590,18 +590,6 @@ nm_setting_ip4_config_new(void) return g_object_new(NM_TYPE_SETTING_IP4_CONFIG, NULL); } -static void -finalize(GObject *object) -{ - NMSettingIP4ConfigPrivate *priv = NM_SETTING_IP4_CONFIG_GET_PRIVATE(object); - - g_free(priv->dhcp_client_id); - g_free(priv->dhcp_fqdn); - g_free(priv->dhcp_vendor_class_identifier); - - G_OBJECT_CLASS(nm_setting_ip4_config_parent_class)->finalize(object); -} - static void nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) { @@ -614,7 +602,6 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; - object_class->finalize = finalize; setting_class->verify = verify; diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index 4b135a05ae..87b9d9581f 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -583,18 +583,6 @@ nm_setting_ip6_config_new(void) return g_object_new(NM_TYPE_SETTING_IP6_CONFIG, NULL); } -static void -finalize(GObject *object) -{ - NMSettingIP6Config * self = NM_SETTING_IP6_CONFIG(object); - NMSettingIP6ConfigPrivate *priv = NM_SETTING_IP6_CONFIG_GET_PRIVATE(self); - - g_free(priv->token); - g_free(priv->dhcp_duid); - - G_OBJECT_CLASS(nm_setting_ip6_config_parent_class)->finalize(object); -} - static void nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) { @@ -607,7 +595,6 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) object_class->get_property = get_property; object_class->set_property = set_property; - object_class->finalize = finalize; setting_class->verify = verify; diff --git a/src/libnm-core-impl/nm-setting-olpc-mesh.c b/src/libnm-core-impl/nm-setting-olpc-mesh.c index 558b266adc..ba612c4f35 100644 --- a/src/libnm-core-impl/nm-setting-olpc-mesh.c +++ b/src/libnm-core-impl/nm-setting-olpc-mesh.c @@ -220,7 +220,6 @@ finalize(GObject *object) if (priv->ssid) g_bytes_unref(priv->ssid); - g_free(priv->dhcp_anycast_addr); G_OBJECT_CLASS(nm_setting_olpc_mesh_parent_class)->finalize(object); } diff --git a/src/libnm-core-impl/nm-setting-ovs-port.c b/src/libnm-core-impl/nm-setting-ovs-port.c index 0b10473d07..fc593268ee 100644 --- a/src/libnm-core-impl/nm-setting-ovs-port.c +++ b/src/libnm-core-impl/nm-setting-ovs-port.c @@ -291,8 +291,7 @@ nm_setting_ovs_port_class_init(NMSettingOvsPortClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingOvsPort:vlan-mode: diff --git a/src/libnm-core-impl/nm-setting-ppp.c b/src/libnm-core-impl/nm-setting-ppp.c index 83708515ce..e4e4951e8a 100644 --- a/src/libnm-core-impl/nm-setting-ppp.c +++ b/src/libnm-core-impl/nm-setting-ppp.c @@ -407,8 +407,7 @@ nm_setting_ppp_class_init(NMSettingPppClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingPpp:noauth: diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 05ea3a70ff..3210c19dac 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -103,21 +103,6 @@ struct _NMSettingClass { guint /* NMSettingParseFlags */ parse_flags, GError ** error); - 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; }; diff --git a/src/libnm-core-impl/nm-setting-tun.c b/src/libnm-core-impl/nm-setting-tun.c index 2698a14541..c26948472b 100644 --- a/src/libnm-core-impl/nm-setting-tun.c +++ b/src/libnm-core-impl/nm-setting-tun.c @@ -228,8 +228,7 @@ nm_setting_tun_class_init(NMSettingTunClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingTun:mode: diff --git a/src/libnm-core-impl/nm-setting-vrf.c b/src/libnm-core-impl/nm-setting-vrf.c index e2707053ef..f998a7dd69 100644 --- a/src/libnm-core-impl/nm-setting-vrf.c +++ b/src/libnm-core-impl/nm-setting-vrf.c @@ -109,8 +109,7 @@ nm_setting_vrf_class_init(NMSettingVrfClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingVrf:table: diff --git a/src/libnm-core-impl/nm-setting-wimax.c b/src/libnm-core-impl/nm-setting-wimax.c index 48a662262f..f7fed08e82 100644 --- a/src/libnm-core-impl/nm-setting-wimax.c +++ b/src/libnm-core-impl/nm-setting-wimax.c @@ -208,7 +208,6 @@ finalize(GObject *object) NMSettingWimaxPrivate *priv = NM_SETTING_WIMAX_GET_PRIVATE(object); g_free(priv->network_name); - g_free(priv->mac_address); G_OBJECT_CLASS(nm_setting_wimax_parent_class)->finalize(object); } diff --git a/src/libnm-core-impl/nm-setting-wired.c b/src/libnm-core-impl/nm-setting-wired.c index 796354444a..8c9bdcd447 100644 --- a/src/libnm-core-impl/nm-setting-wired.c +++ b/src/libnm-core-impl/nm-setting-wired.c @@ -1230,22 +1230,14 @@ finalize(GObject *object) { NMSettingWiredPrivate *priv = NM_SETTING_WIRED_GET_PRIVATE(object); - g_free(priv->port); - g_free(priv->duplex); - g_free(priv->s390_nettype); - _s390_options_clear(priv); - g_free(priv->device_mac_address); g_free(priv->cloned_mac_address); - g_free(priv->generate_mac_address_mask); g_array_unref(priv->mac_address_blacklist); if (priv->s390_subchannels) g_strfreev(priv->s390_subchannels); - g_free(priv->wol_password); - G_OBJECT_CLASS(nm_setting_wired_parent_class)->finalize(object); } diff --git a/src/libnm-core-impl/nm-setting-wireless.c b/src/libnm-core-impl/nm-setting-wireless.c index 7562795491..f15e98efae 100644 --- a/src/libnm-core-impl/nm-setting-wireless.c +++ b/src/libnm-core-impl/nm-setting-wireless.c @@ -1370,8 +1370,6 @@ finalize(GObject *object) if (priv->ssid) g_bytes_unref(priv->ssid); - g_free(priv->bssid); - g_free(priv->device_mac_address); g_free(priv->cloned_mac_address); g_free(priv->generate_mac_address_mask); g_array_unref(priv->mac_address_blacklist); diff --git a/src/libnm-core-impl/nm-setting-wpan.c b/src/libnm-core-impl/nm-setting-wpan.c index 1b556ff7c1..78cdc03650 100644 --- a/src/libnm-core-impl/nm-setting-wpan.c +++ b/src/libnm-core-impl/nm-setting-wpan.c @@ -231,8 +231,7 @@ nm_setting_wpan_class_init(NMSettingWpanClass *klass) object_class->get_property = _nm_setting_property_get_property_direct; object_class->set_property = _nm_setting_property_set_property_direct; - setting_class->verify = verify; - setting_class->finalize_direct = TRUE; + setting_class->verify = verify; /** * NMSettingWpan:mac-address: diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index 1f6992a72f..f5750b7cf1 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -3990,9 +3990,8 @@ constructed(GObject *object) static void finalize(GObject *object) { - NMSetting * self = NM_SETTING(object); - NMSettingPrivate *priv = NM_SETTING_GET_PRIVATE(self); - NMSettingClass * klass = NM_SETTING_GET_CLASS(self); + NMSetting * self = NM_SETTING(object); + NMSettingPrivate *priv = NM_SETTING_GET_PRIVATE(self); if (priv->gendata) { g_free(priv->gendata->names); @@ -4003,8 +4002,7 @@ finalize(GObject *object) G_OBJECT_CLASS(nm_setting_parent_class)->finalize(object); - if (klass->finalize_direct) - _finalize_direct(self); + _finalize_direct(self); } static void