Commit bd8108c3 authored by Thomas Haller's avatar Thomas Haller

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.
parent 6845058c
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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;
}
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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,
......
......@@ -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);
......
......@@ -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);
......
......@@ -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,
......
......@@ -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,
......
......@@ -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;
......
......@@ -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,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment