GNOME.org
 

[network-manager-applet/th/c-e-import-fixes-bgo774290: 8/11] c-e: don't transfer ownership of connection argument for callback functions

commit bd8108c39b5e98c48a541dcb52715b58e26299a2
Author: Thomas Haller <thaller redhat com>
Date:   Fri Nov 11 16:50:31 2016 +0100

    c-e: don't transfer ownership of connection argument for callback functions
    
    Transfering ownership through multiple layers of function calls is
    complicated, because every layer must ensure that it either unrefs
    the connection or passes it to another function that takes ownership.
    
    Just don't do that. Whoever wants to keep a reference on the connection,
    should take it.
    
    Also, note that the destination of the connection is eventually
    nm_connection_editor_new(). But nm_connection_editor_new() doesn't take
    over the reference, it keeps the connection by taking an additional
    reference. I think we just leaked the connection there.
    
    Also, make the connection argument optional where it is not needed.
    That is necessary, because note how vpn_connection_new() behaves
    differently whether we pass in a connection or not.

 src/connection-editor/ce-page.h            |   18 +++++++++++++++++-
 src/connection-editor/connection-helpers.c |    9 +++------
 src/connection-editor/connection-helpers.h |    2 +-
 src/connection-editor/main.c               |    4 +---
 src/connection-editor/nm-connection-list.c |    8 +++++---
 src/connection-editor/page-bluetooth.c     |    5 ++---
 src/connection-editor/page-bond.c          |    4 +++-
 src/connection-editor/page-bridge.c        |    2 ++
 src/connection-editor/page-dsl.c           |    2 ++
 src/connection-editor/page-ethernet.c      |    3 +++
 src/connection-editor/page-infiniband.c    |    3 +++
 src/connection-editor/page-master.c        |    4 +---
 src/connection-editor/page-mobile.c        |   14 +++++++-------
 src/connection-editor/page-team.c          |    4 +++-
 src/connection-editor/page-vlan.c          |    9 +++++----
 src/connection-editor/page-vpn.c           |    2 ++
 src/connection-editor/page-wifi.c          |    2 ++
 17 files changed, 62 insertions(+), 33 deletions(-)
---
diff --git a/src/connection-editor/ce-page.h b/src/connection-editor/ce-page.h
index 911202f..537f468 100644
--- a/src/connection-editor/ce-page.h
+++ b/src/connection-editor/ce-page.h
@@ -40,7 +40,7 @@ struct _func_tag_page_new_connection_result;
 #define FUNC_TAG_PAGE_NEW_CONNECTION_RESULT_IMPL struct _func_tag_page_new_connection_result *_dummy
 #define FUNC_TAG_PAGE_NEW_CONNECTION_RESULT_CALL ((struct _func_tag_page_new_connection_result *) NULL)
 typedef void (*PageNewConnectionResultFunc) (FUNC_TAG_PAGE_NEW_CONNECTION_RESULT_IMPL,
-                                             NMConnection *connection,
+                                             NMConnection *connection, /* allow-none, don't transfer 
reference, allow-keep */
                                              gboolean canceled,
                                              GError *error,
                                              gpointer user_data);
@@ -175,6 +175,22 @@ void ce_page_complete_connection (NMConnection *connection,
                                   gboolean autoconnect,
                                   NMClient *client);
 
+static inline NMConnection *
+_ensure_connection_own (NMConnection **connection)
+{
+       return (*connection) ?: (*connection = nm_simple_connection_new ());
+}
+
+static inline NMConnection *
+_ensure_connection_other (NMConnection *connection, NMConnection **connection_to_free)
+{
+       if (connection) {
+               *connection_to_free = NULL;
+               return connection;
+       }
+       return (*connection_to_free = nm_simple_connection_new ());
+}
+
 CEPage *ce_page_new (GType page_type,
                      NMConnectionEditor *editor,
                      NMConnection *connection,
diff --git a/src/connection-editor/connection-helpers.c b/src/connection-editor/connection-helpers.c
index cfe519f..cfe49e5 100644
--- a/src/connection-editor/connection-helpers.c
+++ b/src/connection-editor/connection-helpers.c
@@ -290,10 +290,7 @@ vpn_connection_import (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
 
        /* The import function decides about the type. */
        g_return_if_fail (!detail);
-
-       /* We're not going to need this one. We'll create another
-        * when we know the file name to import from. */
-       g_object_unref (connection);
+       g_warn_if_fail (!connection);
 
        info = g_slice_new (ImportVpnInfo);
        info->parent = g_object_ref (parent);
@@ -536,7 +533,7 @@ typedef struct {
 
 static void
 new_connection_result (FUNC_TAG_PAGE_NEW_CONNECTION_RESULT_IMPL,
-                       NMConnection *connection,
+                       NMConnection *connection, /* allow-none, don't transfer reference, allow-keep */
                        gboolean canceled,
                        GError *error,
                        gpointer user_data)
@@ -688,7 +685,7 @@ new_connection_dialog_full (GtkWindow *parent_window,
                new_connection_of_type (parent_window,
                                        detail,
                                        detail_data,
-                                       nm_simple_connection_new (),
+                                       NULL,
                                        client,
                                        new_func,
                                        result_func,
diff --git a/src/connection-editor/connection-helpers.h b/src/connection-editor/connection-helpers.h
index db56f30..08fe583 100644
--- a/src/connection-editor/connection-helpers.h
+++ b/src/connection-editor/connection-helpers.h
@@ -45,7 +45,7 @@ struct _func_tag_new_connection_result;
 #define FUNC_TAG_NEW_CONNECTION_RESULT_IMPL struct _func_tag_new_connection_result *_dummy
 #define FUNC_TAG_NEW_CONNECTION_RESULT_CALL ((struct _func_tag_new_connection_result *) NULL)
 typedef void (*NewConnectionResultFunc) (FUNC_TAG_NEW_CONNECTION_RESULT_IMPL,
-                                         NMConnection *connection,
+                                         NMConnection *connection, /* allow-none, don't transfer reference, 
allow-keep */
                                          gpointer user_data);
 
 void new_connection_dialog      (GtkWindow *parent_window,
diff --git a/src/connection-editor/main.c b/src/connection-editor/main.c
index 48cb4bd..b82066f 100644
--- a/src/connection-editor/main.c
+++ b/src/connection-editor/main.c
@@ -61,14 +61,12 @@ idle_create_connection (gpointer user_data)
 {
        CreateConnectionInfo *info = user_data;
 
-       if (!info->connection)
-               info->connection = nm_simple_connection_new ();
        nm_connection_list_create (info->list, info->ctype,
                                   info->detail, info->connection);
 
        g_object_unref (info->list);
        g_free (info->detail);
-       g_object_unref (info->connection);
+       nm_g_object_unref (info->connection);
        g_slice_free (CreateConnectionInfo, info);
        return FALSE;
 }
diff --git a/src/connection-editor/nm-connection-list.c b/src/connection-editor/nm-connection-list.c
index f391df0..37c7eb5 100644
--- a/src/connection-editor/nm-connection-list.c
+++ b/src/connection-editor/nm-connection-list.c
@@ -285,7 +285,6 @@ really_add_connection (FUNC_TAG_NEW_CONNECTION_RESULT_IMPL,
 
        editor = nm_connection_editor_new (GTK_WINDOW (list->dialog), connection, list->client);
        if (!editor) {
-               g_object_unref (connection);
                g_signal_emit (list, list_signals[EDITING_DONE], 0, 0);
                return;
        }
@@ -905,7 +904,10 @@ nm_connection_list_set_type (NMConnectionList *self, GType ctype)
 }
 
 void
-nm_connection_list_create (NMConnectionList *self, GType ctype, const char *detail, NMConnection *connection)
+nm_connection_list_create (NMConnectionList *self,
+                           GType ctype,
+                           const char *detail,
+                           NMConnection *connection)
 {
        ConnectionTypeData *types;
        int i;
@@ -931,7 +933,7 @@ nm_connection_list_create (NMConnectionList *self, GType ctype, const char *deta
                new_connection_of_type (GTK_WINDOW (self->dialog),
                                        detail,
                                        NULL,
-                                       g_object_ref (connection),
+                                       connection,
                                        self->client,
                                        types[i].new_connection_func,
                                        really_add_connection,
diff --git a/src/connection-editor/page-bluetooth.c b/src/connection-editor/page-bluetooth.c
index 6f1de64..b1c41bd 100644
--- a/src/connection-editor/page-bluetooth.c
+++ b/src/connection-editor/page-bluetooth.c
@@ -256,8 +256,7 @@ new_connection_mobile_wizard_done (NMAMobileWizard *wizard,
 
        if (!detail)
                detail = g_strdup (_("Bluetooth connection %d"));
-       if (!info->connection)
-               info->connection = nm_simple_connection_new ();
+       _ensure_connection_own (&info->connection);
        ce_page_complete_connection (info->connection,
                                     detail,
                                     NM_SETTING_BLUETOOTH_SETTING_NAME,
@@ -304,7 +303,7 @@ bluetooth_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        info->client = g_object_ref (client);
        info->user_data = user_data;
        info->type = NM_SETTING_BLUETOOTH_TYPE_PANU;
-       info->connection = g_object_ref (connection);
+       info->connection = nm_g_object_ref (connection);
 
        dialog = gtk_dialog_new_with_buttons (_("Bluetooth Type"),
                                              parent,
diff --git a/src/connection-editor/page-bond.c b/src/connection-editor/page-bond.c
index 2662fb1..5af08b0 100644
--- a/src/connection-editor/page-bond.c
+++ b/src/connection-editor/page-bond.c
@@ -391,7 +391,7 @@ add_slave (CEPageMaster *master, NewConnectionResultFunc result_func)
                new_connection_of_type (priv->toplevel,
                                        NULL,
                                        NULL,
-                                       nm_simple_connection_new (),
+                                       NULL,
                                        CE_PAGE (self)->client,
                                        infiniband_connection_new,
                                        result_func,
@@ -616,7 +616,9 @@ bond_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        NMConnection *conn2;
        const char *iface;
        char *my_iface;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("Bond connection %d"),
                                     NM_SETTING_BOND_SETTING_NAME,
diff --git a/src/connection-editor/page-bridge.c b/src/connection-editor/page-bridge.c
index 2d48bc8..6ebe573 100644
--- a/src/connection-editor/page-bridge.c
+++ b/src/connection-editor/page-bridge.c
@@ -310,7 +310,9 @@ bridge_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        NMConnection *conn2;
        const char *iface;
        char *my_iface;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("Bridge connection %d"),
                                     NM_SETTING_BRIDGE_SETTING_NAME,
diff --git a/src/connection-editor/page-dsl.c b/src/connection-editor/page-dsl.c
index 19833c6..82a5e21 100644
--- a/src/connection-editor/page-dsl.c
+++ b/src/connection-editor/page-dsl.c
@@ -212,7 +212,9 @@ dsl_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                     gpointer user_data)
 {
        NMSetting *setting;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("DSL connection %d"),
                                     NM_SETTING_PPPOE_SETTING_NAME,
diff --git a/src/connection-editor/page-ethernet.c b/src/connection-editor/page-ethernet.c
index e82800c..8593f82 100644
--- a/src/connection-editor/page-ethernet.c
+++ b/src/connection-editor/page-ethernet.c
@@ -515,6 +515,9 @@ ethernet_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                          PageNewConnectionResultFunc result_func,
                          gpointer user_data)
 {
+       gs_unref_object NMConnection *connection_tmp = NULL;
+
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("Ethernet connection %d"),
                                     NM_SETTING_WIRED_SETTING_NAME,
diff --git a/src/connection-editor/page-infiniband.c b/src/connection-editor/page-infiniband.c
index b1a3da1..f3b9845 100644
--- a/src/connection-editor/page-infiniband.c
+++ b/src/connection-editor/page-infiniband.c
@@ -247,6 +247,9 @@ infiniband_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                            PageNewConnectionResultFunc result_func,
                            gpointer user_data)
 {
+       gs_unref_object NMConnection *connection_tmp = NULL;
+
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("InfiniBand connection %d"),
                                     NM_SETTING_INFINIBAND_SETTING_NAME,
diff --git a/src/connection-editor/page-master.c b/src/connection-editor/page-master.c
index 591feec..42d97da 100644
--- a/src/connection-editor/page-master.c
+++ b/src/connection-editor/page-master.c
@@ -399,10 +399,8 @@ add_connection (FUNC_TAG_NEW_CONNECTION_RESULT_IMPL,
        editor = nm_connection_editor_new (priv->toplevel,
                                           connection,
                                           CE_PAGE (self)->client);
-       if (!editor) {
-               g_object_unref (connection);
+       if (!editor)
                return;
-       }
 
        g_signal_connect (editor, "done", G_CALLBACK (add_response_cb), self);
        nm_connection_editor_run (editor);
diff --git a/src/connection-editor/page-mobile.c b/src/connection-editor/page-mobile.c
index aa84a4c..cbe0fb1 100644
--- a/src/connection-editor/page-mobile.c
+++ b/src/connection-editor/page-mobile.c
@@ -477,13 +477,13 @@ new_connection_mobile_wizard_done (NMAMobileWizard *wizard,
                        detail = g_strdup_printf ("%s %s %%d", method->provider_name, method->plan_name);
                else
                        detail = g_strdup_printf ("%s connection %%d", method->provider_name);
-               if (!info->connection)
-                       info->connection = nm_simple_connection_new ();
+
+               _ensure_connection_own (&info->connection);
                ce_page_complete_connection (info->connection,
                                             detail,
-                                             ctype,
-                                             FALSE,
-                                             info->client);
+                                            ctype,
+                                            FALSE,
+                                            info->client);
                g_free (detail);
 
                nm_connection_add_setting (info->connection, type_setting);
@@ -496,7 +496,7 @@ new_connection_mobile_wizard_done (NMAMobileWizard *wizard,
                nma_mobile_wizard_destroy (wizard);
 
        g_object_unref (info->client);
-       g_object_unref (info->connection);
+       nm_g_object_unref (info->connection);
        g_free (info);
 }
 
@@ -527,7 +527,7 @@ mobile_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        info->result_func = result_func;
        info->client = g_object_ref (client);
        info->user_data = user_data;
-       info->connection = g_object_ref (connection);
+       info->connection = nm_g_object_ref (connection);
 
        wizard = nma_mobile_wizard_new (parent, NULL, NM_DEVICE_MODEM_CAPABILITY_NONE, FALSE,
                                        new_connection_mobile_wizard_done, info);
diff --git a/src/connection-editor/page-team.c b/src/connection-editor/page-team.c
index 3bdfbbe..4065202 100644
--- a/src/connection-editor/page-team.c
+++ b/src/connection-editor/page-team.c
@@ -1105,7 +1105,7 @@ add_slave (CEPageMaster *master, NewConnectionResultFunc result_func)
                new_connection_of_type (GTK_WINDOW (toplevel),
                                        NULL,
                                        NULL,
-                                       nm_simple_connection_new (),
+                                       NULL,
                                        CE_PAGE (self)->client,
                                        infiniband_connection_new,
                                        result_func,
@@ -1246,7 +1246,9 @@ team_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        NMConnection *conn2;
        const char *iface;
        char *my_iface;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("Team connection %d"),
                                     NM_SETTING_TEAM_SETTING_NAME,
diff --git a/src/connection-editor/page-vlan.c b/src/connection-editor/page-vlan.c
index a1380c3..fdd3a6b 100644
--- a/src/connection-editor/page-vlan.c
+++ b/src/connection-editor/page-vlan.c
@@ -245,10 +245,8 @@ edit_parent (FUNC_TAG_NEW_CONNECTION_RESULT_IMPL,
        editor = nm_connection_editor_new (priv->toplevel,
                                           connection,
                                           CE_PAGE (self)->client);
-       if (!editor) {
-               g_object_unref (connection);
+       if (!editor)
                return;
-       }
 
        g_signal_connect (editor, "done", G_CALLBACK (edit_parent_cb), self);
        nm_connection_editor_run (editor);
@@ -336,7 +334,7 @@ get_vlan_devices (CEPageVlan *self)
        GSList *devices;
        NMDevice *device;
        int i;
-       
+
        devices_array = nm_client_get_devices (CE_PAGE (self)->client);
        devices = NULL;
        for (i = 0; i < devices_array->len; i++) {
@@ -796,6 +794,9 @@ vlan_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                      PageNewConnectionResultFunc result_func,
                      gpointer user_data)
 {
+       gs_unref_object NMConnection *connection_tmp = NULL;
+
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("VLAN connection %d"),
                                     NM_SETTING_VLAN_SETTING_NAME,
diff --git a/src/connection-editor/page-vpn.c b/src/connection-editor/page-vpn.c
index f0f1b3d..96cfb6c 100644
--- a/src/connection-editor/page-vpn.c
+++ b/src/connection-editor/page-vpn.c
@@ -249,6 +249,7 @@ vpn_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
        gssize split_idx, l;
        const char *add_detail_key = NULL;
        const char *add_detail_val = NULL;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
        if (!detail && !connection) {
                NewVpnInfo *info;
@@ -268,6 +269,7 @@ vpn_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                return;
        }
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        if (detail) {
                service_type = detail;
                add_detail_key = vpn_data ? vpn_data->add_detail_key : NULL;
diff --git a/src/connection-editor/page-wifi.c b/src/connection-editor/page-wifi.c
index f2bb01d..871f451 100644
--- a/src/connection-editor/page-wifi.c
+++ b/src/connection-editor/page-wifi.c
@@ -610,7 +610,9 @@ wifi_connection_new (FUNC_TAG_PAGE_NEW_CONNECTION_IMPL,
                      gpointer user_data)
 {
        NMSetting *s_wifi;
+       gs_unref_object NMConnection *connection_tmp = NULL;
 
+       connection = _ensure_connection_other (connection, &connection_tmp);
        ce_page_complete_connection (connection,
                                     _("Wi-Fi connection %d"),
                                     NM_SETTING_WIRELESS_SETTING_NAME,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]