Skip to content

Commit

Permalink
Fix GVariantBuilder leaks
Browse files Browse the repository at this point in the history
When using g_variant_builder_new(), we must call
g_variant_builder_unref() to free it:
"You should call g_variant_builder_unref() on the return value when it
is no longer needed. The memory will not be automatically freed by any
other call.

In most cases it is easier to place a GVariantBuilder directly on the
stack of the calling function and initialise it with
g_variant_builder_init()."

One of these leaks showed up in valgrind as:
==20702== 16,416 bytes in 114 blocks are definitely lost in loss record 2,114 of 2,115
==20702==    at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20702==    by 0x56EDDF2: g_malloc (gmem.c:97)
==20702==    by 0x570691C: g_slice_alloc (gslice.c:1007)
==20702==    by 0x5729743: g_variant_builder_new (gvariant.c:3169)
==20702==    by 0x40297B: ibus_config_dconf_get_values (config.c:413)
==20702==    by 0x4E44FF2: ibus_config_service_service_method_call (ibusconfigservice.c:214)
==20702==    by 0x4E33249: ibus_service_service_method_call_cb (ibusservice.c:395)
==20702==    by 0x51880D8: call_in_idle_cb (gdbusconnection.c:4875)
==20702==    by 0x56E81D7: g_idle_dispatch (gmain.c:5319)
==20702==    by 0x56E58F1: g_main_dispatch (gmain.c:3064)
==20702==    by 0x56E6667: g_main_context_dispatch (gmain.c:3663)
==20702==    by 0x56E6859: g_main_context_iterate (gmain.c:3734)

BUG=http://code.google.com/p/ibus/issues/detail?id=1712

Review URL: https://codereview.appspot.com/104850044
Patch from Christophe Fergeau <cfergeau@redhat.com>.
  • Loading branch information
cfergeau authored and fujiwarat committed Jun 6, 2014
1 parent 0d17c25 commit 70ddaf1
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 10 deletions.
1 change: 1 addition & 0 deletions bus/ibusimpl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,7 @@ bus_ibus_impl_property_changed (BusIBusImpl *service,
"org.freedesktop.IBus",
builder,
NULL));
g_variant_builder_unref (builder);

bus_dbus_impl_dispatch_message_by_rule (BUS_DEFAULT_DBUS, message, NULL);
g_object_unref (message);
Expand Down
7 changes: 4 additions & 3 deletions bus/inputcontext.c
Original file line number Diff line number Diff line change
Expand Up @@ -710,14 +710,15 @@ bus_input_context_property_changed (BusInputContext *context,
if (context->connection == NULL)
return TRUE;

GVariantBuilder *builder = g_variant_builder_new (G_VARIANT_TYPE_ARRAY);
g_variant_builder_add (builder, "{sv}", property_name, value);
GVariantBuilder builder;
g_variant_builder_init (&builder, G_VARIANT_TYPE_ARRAY);
g_variant_builder_add (&builder, "{sv}", property_name, value);
return bus_input_context_send_signal (context,
"org.freedesktop.DBus.Properties",
"PropertiesChanged",
g_variant_new ("(sa{sv}as)",
"org.freedesktop.IBus",
builder,
&builder,
NULL),
error);
}
Expand Down
8 changes: 4 additions & 4 deletions conf/dconf/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ ibus_config_dconf_get_values (IBusConfigService *config,
gchar *dir, *gdir;
gint len;
gchar **entries, **p;
GVariantBuilder *builder;
GVariantBuilder builder;
gboolean preserve_name;

dir = g_strdup_printf (DCONF_PREFIX"/%s/", section);
Expand All @@ -409,7 +409,7 @@ ibus_config_dconf_get_values (IBusConfigService *config,
preserve_name = _has_prefixes (gdir, dconf->preserve_name_prefixes);

entries = dconf_client_list (client, gdir, &len);
builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
for (p = entries; *p != NULL; p++) {
gchar *gkey = g_strconcat (gdir, *p, NULL);
GVariant *value = dconf_client_read (client, gkey);
Expand All @@ -419,7 +419,7 @@ ibus_config_dconf_get_values (IBusConfigService *config,
if (!preserve_name) {
name = _from_gsettings_name (*p);
}
g_variant_builder_add (builder, "{sv}", name, value);
g_variant_builder_add (&builder, "{sv}", name, value);
if (name != *p) {
g_free (name);
}
Expand All @@ -429,7 +429,7 @@ ibus_config_dconf_get_values (IBusConfigService *config,
g_strfreev (entries);
g_free (gdir);

return g_variant_builder_end (builder);
return g_variant_builder_end (&builder);
}

static gboolean
Expand Down
8 changes: 5 additions & 3 deletions conf/gconf/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -304,19 +304,21 @@ ibus_config_gconf_get_values (IBusConfigService *config,
g_free (dir);

GSList *p;
GVariantBuilder *builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
GVariantBuilder *builder;

g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
for (p = entries; p != NULL; p = p->next) {
GConfEntry *entry = (GConfEntry *)p->data;
if (entry->key != NULL && entry->value != NULL) {
const gchar *name = entry->key + len;
GVariant *value = _from_gconf_value (entry->value);
g_variant_builder_add (builder, "{sv}", name, value);
g_variant_builder_add (&builder, "{sv}", name, value);
}
gconf_entry_free (entry);
}
g_slist_free (entries);

return g_variant_builder_end (builder);
return g_variant_builder_end (&builder);
}

static gboolean
Expand Down
2 changes: 2 additions & 0 deletions src/ibuscomponent.c
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,15 @@ ibus_component_serialize (IBusComponent *component,
g_variant_builder_add (array, "v", ibus_serializable_serialize ((IBusSerializable *)p->data));
}
g_variant_builder_add (builder, "av", array);
g_variant_builder_unref (array);

/* serialize engine desc list */
array = g_variant_builder_new (G_VARIANT_TYPE ("av"));
for (p = component->priv->engines; p != NULL; p = p->next) {
g_variant_builder_add (array, "v", ibus_serializable_serialize ((IBusSerializable *)p->data));
}
g_variant_builder_add (builder, "av", array);
g_variant_builder_unref (array);

return TRUE;
}
Expand Down
1 change: 1 addition & 0 deletions src/ibusengine.c
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,7 @@ ibus_engine_dbus_property_changed (IBusEngine *engine,
IBUS_INTERFACE_ENGINE,
builder,
NULL));
g_variant_builder_unref (builder);

error = NULL;
connection = ibus_service_get_connection ((IBusService *)engine);
Expand Down

0 comments on commit 70ddaf1

Please sign in to comment.