DBus Menu

Merge lp:~chrisccoulson/libdbusmenu/lp722972-part-2 into lp:libdbusmenu/0.5

  1. lp722972-part-2
  2. Merge into trunk
Proposed by Chris Coulson
Status: Merged
Approved by: Ted Gould
Approved revision: 269
Merged at revision: 269
Proposed branch: lp:~chrisccoulson/libdbusmenu/lp722972-part-2
Merge into: lp:libdbusmenu/0.5
Diff against target: 356 lines (+102/-39)
1 file modified
libdbusmenu-glib/client.c (+102/-39)
To merge this branch: bzr merge lp:~chrisccoulson/libdbusmenu/lp722972-part-2
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+53311@code.launchpad.net

Description of the change

The g_variant_get_* family of calls which return a GVariant actually return reference counted, non-floating variant. Ensure that we always keep pointers to these so that we can properly unref them.

The same also applies to g_variant_iter_next_value.

Also fix a couple of other minor leaks along the way. This fixes LP: #722972

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

Wow, looks really good. Thanks!

  review approve
  merge approve

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-glib/client.c'
2--- libdbusmenu-glib/client.c 2011-03-10 16:27:52 +0000
3+++ libdbusmenu-glib/client.c 2011-03-14 19:34:36 +0000
4@@ -598,20 +598,27 @@
5 }
6
7 /* Callback all the folks we can find */
8- GVariantIter * iter = g_variant_iter_new(g_variant_get_child_value(params, 0));
9- GVariant * child;
10+ GVariant * child = g_variant_get_child_value(params, 0);
11+ GVariantIter * iter = g_variant_iter_new(child);
12+ g_variant_unref(child);
13 while ((child = g_variant_iter_next_value(iter)) != NULL) {
14 if (g_strcmp0(g_variant_get_type_string(child), "(ia{sv})") != 0) {
15 g_warning("Properties return signature is not '(ia{sv})' it is '%s'", g_variant_get_type_string(child));
16+ g_variant_unref(child);
17 continue;
18 }
19
20- gint id = g_variant_get_int32(g_variant_get_child_value(child, 0));
21+ GVariant * idv = g_variant_get_child_value(child, 0);
22+ gint id = g_variant_get_int32(idv);
23+ g_variant_unref(idv);
24+
25 GVariant * properties = g_variant_get_child_value(child, 1);
26
27 properties_listener_t * listener = find_listener(listeners, 0, id);
28 if (listener == NULL) {
29 g_warning("Unable to find listener for ID %d", id);
30+ g_variant_unref(properties);
31+ g_variant_unref(child);
32 continue;
33 }
34
35@@ -621,6 +628,8 @@
36 } else {
37 g_warning("Odd, we've already replied to the listener on ID %d", id);
38 }
39+ g_variant_unref(properties);
40+ g_variant_unref(child);
41 }
42 g_variant_iter_free(iter);
43 g_variant_unref(params);
44@@ -676,7 +685,9 @@
45 GVariant * variant_ids = g_variant_builder_end(&builder);
46
47 /* Build up a prop list to pass */
48- g_variant_builder_init(&builder, g_variant_type_new("as"));
49+ GVariantType * type = g_variant_type_new("as");
50+ g_variant_builder_init(&builder, type);
51+ g_variant_type_free(type);
52 /* TODO: need to use delayed property list here */
53 GVariant * variant_props = g_variant_builder_end(&builder);
54
55@@ -1050,12 +1061,13 @@
56 /* Check the text direction if available */
57 GVariant * textdir = g_dbus_proxy_get_cached_property(priv->menuproxy, "TextDirection");
58 if (textdir != NULL) {
59- GVariant * str = textdir;
60- if (g_variant_is_of_type(str, G_VARIANT_TYPE_VARIANT)) {
61- str = g_variant_get_variant(str);
62+ if (g_variant_is_of_type(textdir, G_VARIANT_TYPE_VARIANT)) {
63+ GVariant * tmp = g_variant_get_variant(textdir);
64+ g_variant_unref(textdir);
65+ textdir = tmp;
66 }
67
68- priv->text_direction = dbusmenu_text_direction_get_value_from_nick(g_variant_get_string(str, NULL));
69+ priv->text_direction = dbusmenu_text_direction_get_value_from_nick(g_variant_get_string(textdir, NULL));
70 g_object_notify(G_OBJECT(user_data), DBUSMENU_CLIENT_PROP_TEXT_DIRECTION);
71
72 g_variant_unref(textdir);
73@@ -1063,13 +1075,14 @@
74
75 /* Check the status if available */
76 GVariant * status = g_dbus_proxy_get_cached_property(priv->menuproxy, "Status");
77- if (textdir != NULL) {
78- GVariant * str = status;
79- if (g_variant_is_of_type(str, G_VARIANT_TYPE_VARIANT)) {
80- str = g_variant_get_variant(str);
81+ if (status != NULL) {
82+ if (g_variant_is_of_type(status, G_VARIANT_TYPE_VARIANT)) {
83+ GVariant * tmp = g_variant_get_variant(status);
84+ g_variant_unref(status);
85+ status = tmp;
86 }
87
88- priv->status = dbusmenu_status_get_value_from_nick(g_variant_get_string(str, NULL));
89+ priv->status = dbusmenu_status_get_value_from_nick(g_variant_get_string(status, NULL));
90 g_object_notify(G_OBJECT(user_data), DBUSMENU_CLIENT_PROP_STATUS);
91
92 g_variant_unref(status);
93@@ -1142,20 +1155,22 @@
94 g_variant_iter_init(&iters, properties);
95 while (g_variant_iter_next(&iters, "{sv}", &key, &value)) {
96 if (g_strcmp0(key, "TextDirection") == 0) {
97- GVariant * str = value;
98- if (g_variant_is_of_type(str, G_VARIANT_TYPE_VARIANT)) {
99- str = g_variant_get_variant(str);
100+ if (g_variant_is_of_type(value, G_VARIANT_TYPE_VARIANT)) {
101+ GVariant * tmp = g_variant_get_variant(value);
102+ g_variant_unref(value);
103+ value = tmp;
104 }
105
106- priv->text_direction = dbusmenu_text_direction_get_value_from_nick(g_variant_get_string(str, NULL));
107+ priv->text_direction = dbusmenu_text_direction_get_value_from_nick(g_variant_get_string(value, NULL));
108 }
109 if (g_strcmp0(key, "Status") == 0) {
110- GVariant * str = value;
111- if (g_variant_is_of_type(str, G_VARIANT_TYPE_VARIANT)) {
112- str = g_variant_get_variant(str);
113+ if (g_variant_is_of_type(value, G_VARIANT_TYPE_VARIANT)) {
114+ GVariant * tmp = g_variant_get_variant(value);
115+ g_variant_unref(value);
116+ value = tmp;
117 }
118
119- priv->status = dbusmenu_status_get_value_from_nick(g_variant_get_string(str, NULL));
120+ priv->status = dbusmenu_status_get_value_from_nick(g_variant_get_string(value, NULL));
121 }
122 if (g_strcmp0(key, "IconThemePath") == 0) {
123 if (priv->icon_dirs != NULL) {
124@@ -1224,11 +1239,14 @@
125 /* Remove before adding just incase there is a duplicate, against the
126 rules, but we can handle it so let's do it. */
127 GVariantIter ritems;
128- g_variant_iter_init(&ritems, g_variant_get_child_value(params, 1));
129+ GVariant * ritemsv = g_variant_get_child_value(params, 1);
130+ g_variant_iter_init(&ritems, ritemsv);
131
132 GVariant * ritem;
133 while ((ritem = g_variant_iter_next_value(&ritems)) != NULL) {
134- gint id = g_variant_get_int32(g_variant_get_child_value(ritem, 0));
135+ GVariant * idv = g_variant_get_child_value(ritem, 0);
136+ gint id = g_variant_get_int32(idv);
137+ g_variant_unref(idv);
138 DbusmenuMenuitem * menuitem = dbusmenu_menuitem_find_id(priv->root, id);
139
140 if (menuitem == NULL) {
141@@ -1236,7 +1254,8 @@
142 }
143
144 GVariantIter properties;
145- g_variant_iter_init(&properties, g_variant_get_child_value(ritem, 1));
146+ GVariant * propv = g_variant_get_child_value(ritem, 1);
147+ g_variant_iter_init(&properties, propv);
148 gchar * property;
149
150 while (g_variant_iter_next(&properties, "s", &property)) {
151@@ -1245,16 +1264,23 @@
152 g_free(property);
153 }
154 g_variant_unref(ritem);
155+ g_variant_unref(propv);
156 }
157+ g_variant_unref(ritemsv);
158
159 GVariantIter items;
160- g_variant_iter_init(&items, g_variant_get_child_value(params, 0));
161+ GVariant * itemsv = g_variant_get_child_value(params, 0);
162+ g_variant_iter_init(&items, itemsv);
163
164 GVariant * item;
165 while ((item = g_variant_iter_next_value(&items)) != NULL) {
166- gint id = g_variant_get_int32(g_variant_get_child_value(item, 0));
167+ GVariant * idv = g_variant_get_child_value(item, 0);
168+ gint id = g_variant_get_int32(idv);
169+ g_variant_unref(idv);
170+
171 GVariantIter properties;
172- g_variant_iter_init(&properties, g_variant_get_child_value(item, 1));
173+ GVariant * propv = g_variant_get_child_value(item, 1);
174+ g_variant_iter_init(&properties, propv);
175 gchar * property;
176 GVariant * value;
177
178@@ -1263,14 +1289,21 @@
179 if (G_LIKELY(g_variant_is_of_type(value, G_VARIANT_TYPE_VARIANT))) {
180 /* Unboxing if needed */
181 internalvalue = g_variant_get_variant(value);
182+ g_variant_unref(value);
183 }
184 id_prop_update(proxy, id, property, internalvalue, client);
185+ g_variant_unref(internalvalue);
186 }
187+ g_variant_unref(propv);
188+ g_variant_unref(item);
189 }
190+ g_variant_unref(itemsv);
191 } else if (g_strcmp0(signal, "ItemPropertyUpdated") == 0) {
192 gint id; gchar * property; GVariant * value;
193 g_variant_get(params, "(isv)", &id, &property, &value);
194 id_prop_update(proxy, id, property, value, client);
195+ g_free(property);
196+ g_variant_unref(value);
197 } else if (g_strcmp0(signal, "ItemUpdated") == 0) {
198 gint id;
199 g_variant_get(params, "(i)", &id);
200@@ -1594,7 +1627,9 @@
201 }
202
203 /* First verify and figure out what we've got */
204- gint id = g_variant_get_int32(g_variant_get_child_value(layout, 0));
205+ GVariant * idv = g_variant_get_child_value(layout, 0);
206+ gint id = g_variant_get_int32(idv);
207+ g_variant_unref(idv);
208 if (id < 0) {
209 return NULL;
210 }
211@@ -1607,8 +1642,10 @@
212
213 /* Some variables */
214 GVariantIter children;
215- g_variant_iter_init(&children, g_variant_get_child_value(layout, 2));
216- GVariant * child;
217+ GVariant * childrenv;
218+
219+ childrenv = g_variant_get_child_value(layout, 2);
220+ g_variant_iter_init(&children, childrenv);
221
222 guint position = 0;
223 GList * oldchildren = g_list_copy(dbusmenu_menuitem_get_children(item));
224@@ -1616,16 +1653,22 @@
225
226 /* Go through all the XML Nodes and make sure that we have menuitems
227 to cover those XML nodes. */
228+ GVariant * child;
229 while ((child = g_variant_iter_next_value(&children)) != NULL) {
230 /* g_debug("Looking at child: %d", position); */
231 if (g_variant_is_of_type(child, G_VARIANT_TYPE_VARIANT)) {
232- child = g_variant_get_variant(child);
233+ GVariant * tmp = g_variant_get_variant(child);
234+ g_variant_unref(child);
235+ child = tmp;
236 }
237
238- gint childid = g_variant_get_int32(g_variant_get_child_value(child, 0));
239+ GVariant * childidv = g_variant_get_child_value(child, 0);
240+ gint childid = g_variant_get_int32(childidv);
241+ g_variant_unref(childidv);
242 if (childid < 0) {
243 /* Don't increment the position when there isn't a valid
244 node in the XML tree. It's probably a comment. */
245+ g_variant_unref(child);
246 continue;
247 }
248 DbusmenuMenuitem * childmi = NULL;
249@@ -1665,10 +1708,12 @@
250 GVariantIter iter;
251 gchar * prop;
252 GVariant * value;
253+ GVariant * child_props;
254
255 /* Set the type first as it can manage the behavior of
256 all other properties. */
257- g_variant_iter_init(&iter, g_variant_get_child_value(child, 1));
258+ child_props = g_variant_get_child_value(child, 1);
259+ g_variant_iter_init(&iter, child_props);
260 while (g_variant_iter_next(&iter, "{sv}", &prop, &value)) {
261 if (g_strcmp0(prop, DBUSMENU_MENUITEM_PROP_TYPE) == 0) {
262 dbusmenu_menuitem_property_set_variant(childmi, prop, value);
263@@ -1678,15 +1723,17 @@
264 }
265
266 /* Now go through and do all the properties. */
267- g_variant_iter_init(&iter, g_variant_get_child_value(child, 1));
268+ g_variant_iter_init(&iter, child_props);
269 while (g_variant_iter_next(&iter, "{sv}", &prop, &value)) {
270 dbusmenu_menuitem_property_set_variant(childmi, prop, value);
271 g_free(prop);
272 g_variant_unref(value);
273 }
274+ g_variant_unref(child_props);
275 }
276
277 position++;
278+ g_variant_unref(child);
279 }
280
281 /* Remove any children that are no longer used by this version of
282@@ -1709,19 +1756,24 @@
283 }
284
285 /* now it's time to recurse down the tree. */
286- g_variant_iter_init(&children, g_variant_get_child_value(layout, 2));
287+ g_variant_iter_init(&children, childrenv);
288
289 child = g_variant_iter_next_value(&children);
290 GList * childmis = dbusmenu_menuitem_get_children(item);
291 while (child != NULL && childmis != NULL) {
292 if (g_variant_is_of_type(child, G_VARIANT_TYPE_VARIANT)) {
293- child = g_variant_get_variant(child);
294+ GVariant * tmp = g_variant_get_variant(child);
295+ g_variant_unref(child);
296+ child = tmp;
297 }
298
299- gint xmlid = g_variant_get_int32(g_variant_get_child_value(child, 0));
300+ GVariant * xmlidv = g_variant_get_child_value(child, 0);
301+ gint xmlid = g_variant_get_int32(xmlidv);
302+ g_variant_unref(xmlidv);
303 /* If this isn't a valid menu item we need to move on
304 until we have one. This avoids things like comments. */
305 if (xmlid < 0) {
306+ g_variant_unref(child);
307 child = g_variant_iter_next_value(&children);
308 continue;
309 }
310@@ -1733,10 +1785,13 @@
311
312 parse_layout_xml(client, child, DBUSMENU_MENUITEM(childmis->data), item, proxy);
313
314+ g_variant_unref(child);
315 child = g_variant_iter_next_value(&children);
316 childmis = g_list_next(childmis);
317 }
318
319+ g_variant_unref(childrenv);
320+
321 if (child != NULL) {
322 g_warning("Sync failed, now we've got extra layout nodes.");
323 }
324@@ -1801,6 +1856,7 @@
325
326 GError * error = NULL;
327 GVariant * params = NULL;
328+ GVariant * layout = NULL;
329
330 params = g_dbus_proxy_call_finish(G_DBUS_PROXY(proxy), res, &error);
331
332@@ -1810,8 +1866,11 @@
333 goto out;
334 }
335
336- guint rev = g_variant_get_uint32(g_variant_get_child_value(params, 0));
337- GVariant * layout = g_variant_get_child_value(params, 1);
338+ GVariant * revv = g_variant_get_child_value(params, 0);
339+ guint rev = g_variant_get_uint32(revv);
340+ g_variant_unref(revv);
341+
342+ layout = g_variant_get_child_value(params, 1);
343
344 guint parseable = parse_layout(client, layout);
345
346@@ -1839,6 +1898,10 @@
347 priv->layoutcall = NULL;
348 }
349
350+ if (layout != NULL) {
351+ g_variant_unref(layout);
352+ }
353+
354 if (params != NULL) {
355 g_variant_unref(params);
356 }

Subscribers

People subscribed via source and target branches