Skip to content

Commit

Permalink
Use g_auto and G_VARIANT_BUILDER_INIT for all GVariantBuilders
Browse files Browse the repository at this point in the history 
One has to be very careful about the control flow and early exits
without the g_auto which can easily result in memory leaks. I noticed at
least 5 cases where the change does fix a leak, even though it might be
a bit hard to trigger.

This also makes sure that all g_auto usages are initialized correctly.
  • Loading branch information
@swick @GeorgesStavracas
swick authored and GeorgesStavracas committed Nov 5, 2024
1 parent 89a2e21 commit debd30e
Show file tree
Hide file tree
Showing 21 changed files with 153 additions and 208 deletions.
4 changes: 2 additions & 2 deletions src/account.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ handle_get_user_information (XdpDbusAccount *object,
const char *app_id = xdp_app_info_get_id (request->app_info);
g_autoptr(GError) error = NULL;
g_autoptr(XdpDbusImplRequest) impl_request = NULL;
GVariantBuilder options;
g_auto(GVariantBuilder) options =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

g_debug ("Handling GetUserInformation");

Expand All @@ -203,7 +204,6 @@ handle_get_user_information (XdpDbusAccount *object,
request_set_impl_request (request, impl_request);
request_export (request, g_dbus_method_invocation_get_connection (invocation));

g_variant_builder_init (&options, G_VARIANT_TYPE_VARDICT);
xdp_filter_options (arg_options, &options,
user_information_options, G_N_ELEMENTS (user_information_options),
NULL);
Expand Down
21 changes: 10 additions & 11 deletions src/background.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,18 +352,18 @@ remove_outdated_instances (int stamp)
static void
update_background_monitor_properties (void)
{
GVariantBuilder builder;
g_auto(GVariantBuilder) builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE ("aa{sv}"));
GHashTableIter iter;
InstanceData *data;
char *id;

g_variant_builder_init (&builder, G_VARIANT_TYPE ("aa{sv}"));

G_LOCK (applications);
g_hash_table_iter_init (&iter, applications);
while (g_hash_table_iter_next (&iter, (gpointer *)&id, (gpointer *)&data))
{
GVariantBuilder app_builder;
g_auto(GVariantBuilder) app_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
const char *app_id;
const char *id;

Expand All @@ -377,7 +377,6 @@ update_background_monitor_properties (void)
app_id = flatpak_instance_get_app (data->instance);
g_assert (app_id != NULL);

g_variant_builder_init (&app_builder, G_VARIANT_TYPE_VARDICT);
g_variant_builder_add (&app_builder, "{sv}", "app_id", g_variant_new_string (app_id));
g_variant_builder_add (&app_builder, "{sv}", "instance", g_variant_new_string (id));
if (data->status_message)
Expand Down Expand Up @@ -811,7 +810,8 @@ handle_request_background_in_thread_func (GTask *task,

if (permission == PERMISSION_ASK)
{
GVariantBuilder opt_builder;
g_auto(GVariantBuilder) opt_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autofree char *app_id = NULL;
g_autofree char *title = NULL;
g_autofree char *subtitle = NULL;
Expand All @@ -835,7 +835,6 @@ handle_request_background_in_thread_func (GTask *task,

g_debug ("Calling backend for background access for: %s", id);

g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT);
g_variant_builder_add (&opt_builder, "{sv}", "deny_label", g_variant_new_string (_("Don't allow")));
g_variant_builder_add (&opt_builder, "{sv}", "grant_label", g_variant_new_string (_("Allow")));
if (!xdp_dbus_impl_access_call_access_dialog_sync (access_impl,
Expand Down Expand Up @@ -883,9 +882,9 @@ handle_request_background_in_thread_func (GTask *task,
if (request->exported)
{
XdgDesktopPortalResponseEnum portal_response;
GVariantBuilder results;
g_auto(GVariantBuilder) results =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

g_variant_builder_init (&results, G_VARIANT_TYPE_VARDICT);
g_variant_builder_add (&results, "{sv}", "background", g_variant_new_boolean (allowed));
g_variant_builder_add (&results, "{sv}", "autostart", g_variant_new_boolean (autostart_enabled));

Expand Down Expand Up @@ -1160,7 +1159,8 @@ handle_set_status (XdpDbusBackground *object,
g_autoptr(GVariant) options = NULL;
g_autoptr(GError) error = NULL;
g_autoptr(GTask) task = NULL;
GVariantBuilder opt_builder;
g_auto(GVariantBuilder) opt_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
const char *id = NULL;
Call *call;

Expand All @@ -1187,7 +1187,6 @@ handle_set_status (XdpDbusBackground *object,
return G_DBUS_METHOD_INVOCATION_HANDLED;
}

g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT);
if (!xdp_filter_options (arg_options, &opt_builder,
set_status_options,
G_N_ELEMENTS (set_status_options),
Expand Down
9 changes: 4 additions & 5 deletions src/camera.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ query_permission_sync (Request *request)
permission = get_permission_sync (app_id, PERMISSION_TABLE, PERMISSION_DEVICE_CAMERA);
if (permission == PERMISSION_ASK || permission == PERMISSION_UNSET)
{
GVariantBuilder opt_builder;
g_auto(GVariantBuilder) opt_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autofree char *title = NULL;
g_autofree char *body = NULL;
guint32 response = 2;
Expand All @@ -98,7 +99,6 @@ query_permission_sync (Request *request)
info = (GAppInfo*)g_desktop_app_info_new (desktop_id);
}

g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT);
g_variant_builder_add (&opt_builder, "{sv}", "icon", g_variant_new_string ("camera-web-symbolic"));

if (info)
Expand Down Expand Up @@ -173,11 +173,10 @@ handle_access_camera_in_thread_func (GTask *task,

if (request->exported)
{
GVariantBuilder results;
g_auto(GVariantBuilder) results =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
guint32 response;

g_variant_builder_init (&results, G_VARIANT_TYPE_VARDICT);

response = allowed ? XDG_DESKTOP_PORTAL_RESPONSE_SUCCESS
: XDG_DESKTOP_PORTAL_RESPONSE_CANCELLED;
g_debug ("Camera: sending response %d", response);
Expand Down
4 changes: 2 additions & 2 deletions src/clipboard.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ handle_set_selection (XdpDbusClipboard *object,
{
Call *call = call_from_invocation (invocation);
Session *session;
GVariantBuilder options_builder;
g_auto(GVariantBuilder) options_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autoptr(GVariant) options = NULL;
g_autoptr(GError) error = NULL;

Expand Down Expand Up @@ -148,7 +149,6 @@ handle_set_selection (XdpDbusClipboard *object,
return G_DBUS_METHOD_INVOCATION_HANDLED;
}

g_variant_builder_init (&options_builder, G_VARIANT_TYPE_VARDICT);
if (!xdp_filter_options (arg_options,
&options_builder,
clipboard_set_selection_options,
Expand Down
13 changes: 4 additions & 9 deletions src/dynamic-launcher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,12 +481,11 @@ prepare_install_done (GObject *source,
guint response = 2;
g_autoptr(GVariant) results = NULL;
g_autoptr(GError) error = NULL;
GVariantBuilder results_builder;
g_auto(GVariantBuilder) results_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

REQUEST_AUTOLOCK (request);

g_variant_builder_init (&results_builder, G_VARIANT_TYPE_VARDICT);

if (!xdp_dbus_impl_dynamic_launcher_call_prepare_install_finish (XDP_DBUS_IMPL_DYNAMIC_LAUNCHER (source),
&response,
&results,
Expand Down Expand Up @@ -542,10 +541,6 @@ prepare_install_done (GObject *source,

request_unexport (request);
}
else
{
g_variant_builder_clear (&results_builder);
}
}

static gboolean
Expand Down Expand Up @@ -620,7 +615,8 @@ handle_prepare_install (XdpDbusDynamicLauncher *object,
const char *app_id = xdp_app_info_get_id (request->app_info);
g_autoptr(GError) error = NULL;
g_autoptr(XdpDbusImplRequest) impl_request = NULL;
GVariantBuilder opt_builder;
g_auto(GVariantBuilder) opt_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autofree char *icon_format = NULL;
g_autofree char *icon_size = NULL;
g_autoptr(GVariant) icon_v = NULL;
Expand All @@ -641,7 +637,6 @@ handle_prepare_install (XdpDbusDynamicLauncher *object,
request_set_impl_request (request, impl_request);
request_export (request, g_dbus_method_invocation_get_connection (invocation));

g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT);
if (!xdp_filter_options (arg_options, &opt_builder,
prepare_install_options, G_N_ELEMENTS (prepare_install_options), &error))
{
Expand Down
8 changes: 4 additions & 4 deletions src/email.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ send_response_in_thread_func (GTask *task,

if (request->exported)
{
GVariantBuilder new_results;
g_auto(GVariantBuilder) new_results =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

g_variant_builder_init (&new_results, G_VARIANT_TYPE_VARDICT);
xdp_dbus_request_emit_response (XDP_DBUS_REQUEST (request),
response,
g_variant_builder_end (&new_results));
Expand Down Expand Up @@ -235,10 +235,10 @@ handle_compose_email (XdpDbusEmail *object,
attachment_fds = g_variant_lookup_value (arg_options, "attachment_fds", G_VARIANT_TYPE ("ah"));
if (attachment_fds)
{
GVariantBuilder attachments;
g_auto(GVariantBuilder) attachments =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_STRING_ARRAY);
int i;

g_variant_builder_init (&attachments, G_VARIANT_TYPE_STRING_ARRAY);
for (i = 0; i < g_variant_n_children (attachment_fds); i++)
{
g_autofree char *path = NULL;
Expand Down
26 changes: 10 additions & 16 deletions src/global-shortcuts.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ session_created_cb (GObject *source_object,
guint response = 2;
g_autoptr (GVariant) results = NULL;
gboolean should_close_session;
GVariantBuilder results_builder;
g_auto(GVariantBuilder) results_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autoptr(GError) error = NULL;

REQUEST_AUTOLOCK (request);
Expand All @@ -168,8 +169,6 @@ session_created_cb (GObject *source_object,
SESSION_AUTOLOCK_UNREF (g_object_ref (session));
g_object_set_qdata (G_OBJECT (request), quark_request_session, NULL);

g_variant_builder_init (&results_builder, G_VARIANT_TYPE_VARDICT);

if (!xdp_dbus_impl_global_shortcuts_call_create_session_finish (impl,
&response,
&results,
Expand Down Expand Up @@ -211,10 +210,6 @@ session_created_cb (GObject *source_object,
g_variant_builder_end (&results_builder));
request_unexport (request);
}
else
{
g_variant_builder_clear (&results_builder);
}

if (should_close_session)
session_close (session, FALSE);
Expand Down Expand Up @@ -318,9 +313,9 @@ shortcuts_bound_cb (GObject *source_object,
{
if (!results)
{
GVariantBuilder results_builder;
g_auto(GVariantBuilder) results_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

g_variant_builder_init (&results_builder, G_VARIANT_TYPE_VARDICT);
results = g_variant_ref_sink (g_variant_builder_end (&results_builder));
}

Expand All @@ -346,7 +341,8 @@ xdp_verify_shortcuts (GVariant *shortcuts,
iter = g_variant_iter_new (shortcuts);
while (g_variant_iter_loop (iter, "(s@a{sv})", &shortcut_name, &values))
{
GVariantBuilder shortcut_builder;
g_auto(GVariantBuilder) shortcut_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

if (shortcut_name[0] == 0)
{
Expand All @@ -357,7 +353,6 @@ xdp_verify_shortcuts (GVariant *shortcuts,
return FALSE;
}

g_variant_builder_init (&shortcut_builder, G_VARIANT_TYPE_VARDICT);
if (!xdp_filter_options (values, &shortcut_builder,
global_shortcuts_keys,
G_N_ELEMENTS (global_shortcuts_keys),
Expand Down Expand Up @@ -488,9 +483,9 @@ shortcuts_listed_cb (GObject *source_object,
{
if (!results)
{
GVariantBuilder results_builder;
g_auto(GVariantBuilder) results_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

g_variant_builder_init (&results_builder, G_VARIANT_TYPE_VARDICT);
results = g_variant_ref_sink (g_variant_builder_end (&results_builder));
}

Expand All @@ -513,18 +508,17 @@ handle_list_shortcuts (XdpDbusGlobalShortcuts *object,
Session *session;
g_autoptr(XdpDbusImplRequest) impl_request = NULL;
g_autoptr(GError) error = NULL;
GVariantBuilder options_builder;
g_auto(GVariantBuilder) options_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autoptr(GVariant) options = NULL;

REQUEST_AUTOLOCK (request);

g_variant_builder_init (&options_builder, G_VARIANT_TYPE_VARDICT);
if (!xdp_filter_options (arg_options, &options_builder,
global_shortcuts_list_shortcuts_options,
G_N_ELEMENTS (global_shortcuts_list_shortcuts_options),
&error))
{
g_variant_builder_clear (&options_builder);
g_dbus_method_invocation_return_gerror (invocation, error);
return G_DBUS_METHOD_INVOCATION_HANDLED;
}
Expand Down
18 changes: 6 additions & 12 deletions src/inhibit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ inhibit_done (GObject *source,

if (request->exported)
{
GVariantBuilder new_results;

g_variant_builder_init (&new_results, G_VARIANT_TYPE_VARDICT);
g_auto(GVariantBuilder) new_results =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);

xdp_dbus_request_emit_response (XDP_DBUS_REQUEST (request),
response,
Expand Down Expand Up @@ -201,7 +200,8 @@ handle_inhibit (XdpDbusInhibit *object,
g_autoptr(GError) error = NULL;
g_autoptr(XdpDbusImplRequest) impl_request = NULL;
g_autoptr(GTask) task = NULL;
GVariantBuilder opt_builder;
g_auto(GVariantBuilder) opt_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autoptr(GVariant) options = NULL;

REQUEST_AUTOLOCK (request);
Expand All @@ -215,7 +215,6 @@ handle_inhibit (XdpDbusInhibit *object,
return G_DBUS_METHOD_INVOCATION_HANDLED;
}

g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT);
xdp_filter_options (arg_options, &opt_builder,
inhibit_options, G_N_ELEMENTS (inhibit_options),
NULL);
Expand Down Expand Up @@ -348,7 +347,8 @@ create_monitor_done (GObject *source_object,
Session *session;
guint response = 2;
gboolean should_close_session;
GVariantBuilder results_builder;
g_auto(GVariantBuilder) results_builder =
G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
g_autoptr(GError) error = NULL;

REQUEST_AUTOLOCK (request);
Expand All @@ -357,8 +357,6 @@ create_monitor_done (GObject *source_object,
SESSION_AUTOLOCK_UNREF (g_object_ref (session));
g_object_set_data (G_OBJECT (request), "session", NULL);

g_variant_builder_init (&results_builder, G_VARIANT_TYPE_VARDICT);

if (!xdp_dbus_impl_inhibit_call_create_monitor_finish (impl, &response, res, &error))
{
g_dbus_error_strip_remote_error (error);
Expand Down Expand Up @@ -396,10 +394,6 @@ create_monitor_done (GObject *source_object,
g_variant_builder_end (&results_builder));
request_unexport (request);
}
else
{
g_variant_builder_clear (&results_builder);
}

if (should_close_session)
session_close (session, FALSE);
Expand Down
Loading

0 comments on commit debd30e

Please sign in to comment.