From 72a98c5b29ef53c054bffc53ee89b9d3b35a520b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 5 Oct 2009 17:46:16 -0700 Subject: [PATCH] system-settings: fix crash saving default wired connections to backing storage (lp:430781) Fix up refcounting, plus it turns out that we already have the MAC address lying around as a GObject data item, so we don't need to go grab it from the connection itself. --- libnm-glib/nm-exported-connection.c | 4 + .../nm-default-wired-connection.c | 77 +++---------------- src/system-settings/nm-sysconfig-settings.c | 9 +++ 3 files changed, 25 insertions(+), 65 deletions(-) diff --git a/libnm-glib/nm-exported-connection.c b/libnm-glib/nm-exported-connection.c index 43df56bb59..47fd8644d8 100644 --- a/libnm-glib/nm-exported-connection.c +++ b/libnm-glib/nm-exported-connection.c @@ -134,8 +134,10 @@ update (NMSettingsConnectionInterface *connection, NMSettingsConnectionInterfaceUpdateFunc callback, gpointer user_data) { + g_object_ref (connection); nm_settings_connection_interface_emit_updated (connection); callback (connection, NULL, user_data); + g_object_unref (connection); return TRUE; } @@ -183,8 +185,10 @@ do_delete (NMSettingsConnectionInterface *connection, NMSettingsConnectionInterfaceDeleteFunc callback, gpointer user_data) { + g_object_ref (connection); g_signal_emit_by_name (connection, "removed"); callback (connection, NULL, user_data); + g_object_unref (connection); return TRUE; } diff --git a/src/system-settings/nm-default-wired-connection.c b/src/system-settings/nm-default-wired-connection.c index 0fbd40bd4b..54e00d6273 100644 --- a/src/system-settings/nm-default-wired-connection.c +++ b/src/system-settings/nm-default-wired-connection.c @@ -91,51 +91,21 @@ nm_default_wired_connection_get_device (NMDefaultWiredConnection *wired) return NM_DEFAULT_WIRED_CONNECTION_GET_PRIVATE (wired)->device; } -static GByteArray * -dup_wired_mac (NMConnection *connection) -{ - NMSettingWired *s_wired; - const GByteArray *mac; - GByteArray *dup; - - s_wired = (NMSettingWired *) nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED); - if (!s_wired) - return NULL; - - mac = nm_setting_wired_get_mac_address (s_wired); - if (!mac || (mac->len != ETH_ALEN)) - return NULL; - - dup = g_byte_array_sized_new (ETH_ALEN); - g_byte_array_append (dup, mac->data, ETH_ALEN); - return dup; -} - static gboolean update (NMSettingsConnectionInterface *connection, NMSettingsConnectionInterfaceUpdateFunc callback, gpointer user_data) { NMDefaultWiredConnection *self = NM_DEFAULT_WIRED_CONNECTION (connection); - GByteArray *mac; - gboolean failed = FALSE; - /* Ensure object stays alive across signal emission */ - g_object_ref (self); - - /* Save a copy of the current MAC address just in case the user - * changed it when updating the connection. + /* Keep the object alive over try-update since it might get removed + * from the settings service there, but we still need it for the callback. */ - mac = dup_wired_mac (NM_CONNECTION (self)); - - g_signal_emit (self, signals[TRY_UPDATE], 0, &failed); - if (!failed) - g_signal_emit (connection, signals[DELETED], 0, mac); - - g_byte_array_free (mac, TRUE); - g_object_unref (self); - - return parent_settings_connection_iface->update (connection, callback, user_data); + g_object_ref (connection); + g_signal_emit (self, signals[TRY_UPDATE], 0); + callback (connection, NULL, user_data); + g_object_unref (connection); + return TRUE; } static gboolean @@ -144,16 +114,9 @@ do_delete (NMSettingsConnectionInterface *connection, gpointer user_data) { NMDefaultWiredConnection *self = NM_DEFAULT_WIRED_CONNECTION (connection); - GByteArray *mac; - - g_object_ref (self); - mac = dup_wired_mac (NM_CONNECTION (self)); - - g_signal_emit (self, signals[DELETED], 0, mac); - - g_byte_array_free (mac, TRUE); - g_object_unref (self); + NMDefaultWiredConnectionPrivate *priv = NM_DEFAULT_WIRED_CONNECTION_GET_PRIVATE (connection); + g_signal_emit (self, signals[DELETED], 0, priv->mac); return parent_settings_connection_iface->delete (connection, callback, user_data); } @@ -283,22 +246,6 @@ set_property (GObject *object, guint prop_id, } } -static gboolean -try_update_signal_accumulator (GSignalInvocationHint *ihint, - GValue *return_accu, - const GValue *handler_return, - gpointer data) -{ - if (g_value_get_boolean (handler_return)) { - g_value_set_boolean (return_accu, TRUE); - /* Stop */ - return FALSE; - } - - /* Continue if handler didn't fail */ - return TRUE; -} - static void nm_default_wired_connection_class_init (NMDefaultWiredConnectionClass *klass) { @@ -341,9 +288,9 @@ nm_default_wired_connection_class_init (NMDefaultWiredConnectionClass *klass) g_signal_new ("try-update", G_OBJECT_CLASS_TYPE (object_class), G_SIGNAL_RUN_LAST, - 0, try_update_signal_accumulator, NULL, - _nm_marshal_BOOLEAN__VOID, - G_TYPE_BOOLEAN, 0); + 0, NULL, NULL, + g_cclosure_marshal_VOID__VOID, + G_TYPE_NONE, 0); /* The 'deleted' signal is used to signal intentional deletions (like * updating or user-requested deletion) rather than using the diff --git a/src/system-settings/nm-sysconfig-settings.c b/src/system-settings/nm-sysconfig-settings.c index 0f50364091..32895be169 100644 --- a/src/system-settings/nm-sysconfig-settings.c +++ b/src/system-settings/nm-sysconfig-settings.c @@ -1109,6 +1109,11 @@ cleanup: NULL); } +static void +delete_cb (NMSettingsConnectionInterface *connection, GError *error, gpointer user_data) +{ +} + static gboolean default_wired_try_update (NMDefaultWiredConnection *wired, NMSysconfigSettings *self) @@ -1129,6 +1134,10 @@ default_wired_try_update (NMDefaultWiredConnection *wired, remove_connection (self, NM_SETTINGS_CONNECTION_INTERFACE (wired), FALSE); if (add_new_connection (self, NM_CONNECTION (wired), &error)) { + nm_settings_connection_interface_delete (NM_SETTINGS_CONNECTION_INTERFACE (wired), + delete_cb, + NULL); + g_object_set_data (G_OBJECT (nm_default_wired_connection_get_device (wired)), DEFAULT_WIRED_TAG, NULL);