gdbusconnection: Fix user_data leaks on error

There were a couple of functions in `GDBusConnection` which take a
`user_data` argument, but which then leak it if they error out early.

A true positive spotted by scan-build!

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Helps: #1767
This commit is contained in:
Philip Withnall 2024-04-10 00:44:40 +01:00
parent 39f0e6d435
commit 747e3af998
No known key found for this signature in database
GPG Key ID: DCDF5885B1F3ED73
2 changed files with 18 additions and 7 deletions

View File

@ -5364,6 +5364,9 @@ g_dbus_connection_register_object (GDBusConnection *connection,
out: out:
CONNECTION_UNLOCK (connection); CONNECTION_UNLOCK (connection);
if (ret == 0 && user_data_free_func != NULL)
user_data_free_func (user_data);
return ret; return ret;
} }
@ -7020,6 +7023,9 @@ g_dbus_connection_register_subtree (GDBusConnection *connection,
out: out:
CONNECTION_UNLOCK (connection); CONNECTION_UNLOCK (connection);
if (ret == 0 && user_data_free_func != NULL)
user_data_free_func (user_data);
return ret; return ret;
} }

View File

@ -1003,12 +1003,14 @@ test_object_registration (void)
guint non_subtree_object_path_bar_reg_id; guint non_subtree_object_path_bar_reg_id;
guint dyna_subtree_registration_id; guint dyna_subtree_registration_id;
guint num_successful_registrations; guint num_successful_registrations;
guint num_failed_registrations;
data.num_unregistered_calls = 0; data.num_unregistered_calls = 0;
data.num_unregistered_subtree_calls = 0; data.num_unregistered_subtree_calls = 0;
data.num_subtree_nodes = 0; data.num_subtree_nodes = 0;
num_successful_registrations = 0; num_successful_registrations = 0;
num_failed_registrations = 0;
error = NULL; error = NULL;
c = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); c = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error);
@ -1100,7 +1102,8 @@ test_object_registration (void)
intern2_bar_reg_id = registration_id; intern2_bar_reg_id = registration_id;
num_successful_registrations++; num_successful_registrations++;
/* register at the same path/interface - this should fail */ /* register at the same path/interface - this should fail and result in an
* immediate unregistration (so the user_data isnt leaked) */
registration_id = g_dbus_connection_register_object (c, registration_id = g_dbus_connection_register_object (c,
"/foo/boss/interns/intern2", "/foo/boss/interns/intern2",
(GDBusInterfaceInfo *) &bar_interface_info, (GDBusInterfaceInfo *) &bar_interface_info,
@ -1113,6 +1116,8 @@ test_object_registration (void)
g_error_free (error); g_error_free (error);
error = NULL; error = NULL;
g_assert (registration_id == 0); g_assert (registration_id == 0);
g_assert_cmpint (data.num_unregistered_calls, ==, 1);
num_failed_registrations++;
/* register at different interface - shouldn't fail */ /* register at different interface - shouldn't fail */
registration_id = g_dbus_connection_register_object (c, registration_id = g_dbus_connection_register_object (c,
@ -1130,7 +1135,7 @@ test_object_registration (void)
/* unregister it via the id */ /* unregister it via the id */
g_assert (g_dbus_connection_unregister_object (c, registration_id)); g_assert (g_dbus_connection_unregister_object (c, registration_id));
g_main_context_iteration (NULL, FALSE); g_main_context_iteration (NULL, FALSE);
g_assert_cmpint (data.num_unregistered_calls, ==, 1); g_assert_cmpint (data.num_unregistered_calls, ==, 2);
intern2_foo_reg_id = 0; intern2_foo_reg_id = 0;
/* register it back */ /* register it back */
@ -1181,12 +1186,12 @@ test_object_registration (void)
g_error_free (error); g_error_free (error);
error = NULL; error = NULL;
g_assert (registration_id == 0); g_assert (registration_id == 0);
g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 1);
/* unregister it, then register it again */ /* unregister it, then register it again */
g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 0);
g_assert (g_dbus_connection_unregister_subtree (c, subtree_registration_id)); g_assert (g_dbus_connection_unregister_subtree (c, subtree_registration_id));
g_main_context_iteration (NULL, FALSE); g_main_context_iteration (NULL, FALSE);
g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 1); g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 2);
subtree_registration_id = g_dbus_connection_register_subtree (c, subtree_registration_id = g_dbus_connection_register_subtree (c,
"/foo/boss/executives", "/foo/boss/executives",
&subtree_vtable, &subtree_vtable,
@ -1382,10 +1387,10 @@ test_object_registration (void)
#endif #endif
/* check that unregistering the subtree handler works */ /* check that unregistering the subtree handler works */
g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 1); g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 2);
g_assert (g_dbus_connection_unregister_subtree (c, subtree_registration_id)); g_assert (g_dbus_connection_unregister_subtree (c, subtree_registration_id));
g_main_context_iteration (NULL, FALSE); g_main_context_iteration (NULL, FALSE);
g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 2); g_assert_cmpint (data.num_unregistered_subtree_calls, ==, 3);
nodes = get_nodes_at (c, "/foo/boss/executives"); nodes = get_nodes_at (c, "/foo/boss/executives");
g_assert (nodes != NULL); g_assert (nodes != NULL);
g_assert_cmpint (g_strv_length (nodes), ==, 1); g_assert_cmpint (g_strv_length (nodes), ==, 1);
@ -1405,7 +1410,7 @@ test_object_registration (void)
g_assert (g_dbus_connection_unregister_object (c, non_subtree_object_path_foo_reg_id)); g_assert (g_dbus_connection_unregister_object (c, non_subtree_object_path_foo_reg_id));
g_main_context_iteration (NULL, FALSE); g_main_context_iteration (NULL, FALSE);
g_assert_cmpint (data.num_unregistered_calls, ==, num_successful_registrations); g_assert_cmpint (data.num_unregistered_calls, ==, num_successful_registrations + num_failed_registrations);
/* check that we no longer export any objects - TODO: it looks like there's a bug in /* check that we no longer export any objects - TODO: it looks like there's a bug in
* libdbus-1 here: libdbus still reports the '/foo' object; so disable the test for now * libdbus-1 here: libdbus still reports the '/foo' object; so disable the test for now