BAMF

Merge lp:~3v1n0/bamf/valgrind-fixes into lp:bamf/0.4

  1. valgrind-fixes
  2. Merge into 0.4
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Jason Smith
Approved revision: 466
Merged at revision: 454
Proposed branch: lp:~3v1n0/bamf/valgrind-fixes
Merge into: lp:bamf/0.4
Diff against target: 448 lines (+95/-78)
4 files modified
src/bamf-application.c (+2/-2)
src/bamf-matcher.c (+78/-63)
src/bamf-view.c (+8/-7)
src/bamf-view.h (+7/-6)
To merge this branch: bzr merge lp:~3v1n0/bamf/valgrind-fixes
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve
Review via email: mp+98007@code.launchpad.net

Commit message

Fix memory leaks and some read errors in Bamf

Description of the change

Fix some BAMF read errors and memory leaks.

To post a comment you must log in.
lp:~3v1n0/bamf/valgrind-fixes updated
463. By Marco Trevisan (Treviño)

BamfMatcher: cleanup bamf_matcher_unregister_view

464. By Marco Trevisan (Treviño)

BamfMatcher: moving to g_list_find + g_list_delete_link

465. By Marco Trevisan (Treviño)

BamfMatcher: use a while loop in dispose function, to unregister views.

466. By Marco Trevisan (Treviño)

BamfMatcher: rename bamf_matcher_register_view into bamf_matcher_register_view_stealing_ref

Revision history for this message
Michal Hruby (mhr3) wrote :

I can't say I like too much the stealing behavior, but at least it's super obvious that it steals the ref, other than that it's looking good, yey for fewer leaks :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/bamf-application.c'
2--- src/bamf-application.c 2012-01-12 16:30:44 +0000
3+++ src/bamf-application.c 2012-03-19 17:14:17 +0000
4@@ -49,12 +49,12 @@
5
6 #define STUB_KEY "X-Ayatana-Appmenu-Show-Stubs"
7
8-static char *
9+static const char *
10 bamf_application_get_icon (BamfView *view)
11 {
12 g_return_val_if_fail (BAMF_IS_APPLICATION (view), NULL);
13
14- return g_strdup (BAMF_APPLICATION (view)->priv->icon);
15+ return BAMF_APPLICATION (view)->priv->icon;
16 }
17
18 char *
19
20=== modified file 'src/bamf-matcher.c'
21--- src/bamf-matcher.c 2012-02-10 20:05:54 +0000
22+++ src/bamf-matcher.c 2012-03-19 17:14:17 +0000
23@@ -46,6 +46,7 @@
24 VIEW_REMOVED
25 } ViewChangeType;
26
27+static BamfMatcher *static_matcher;
28 static guint matcher_signals[LAST_SIGNAL] = { 0 };
29
30 struct _BamfMatcherPrivate
31@@ -117,14 +118,13 @@
32 else
33 priv->active_win = NULL;
34
35-
36 g_signal_emit_by_name (matcher, "active-window-changed",
37 BAMF_IS_VIEW (last) ? bamf_view_get_path (BAMF_VIEW (last)) : "",
38 BAMF_IS_VIEW (priv->active_win) ? bamf_view_get_path (BAMF_VIEW (priv->active_win)) : "");
39 }
40 }
41
42-static void bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view, gboolean unref);
43+static void bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view);
44
45 static gboolean
46 emit_paths_changed (gpointer user_data)
47@@ -229,11 +229,11 @@
48 static void
49 on_view_closed (BamfView *view, BamfMatcher *self)
50 {
51- bamf_matcher_unregister_view (self, view, TRUE);
52+ bamf_matcher_unregister_view (self, view);
53 }
54
55 static void
56-bamf_matcher_register_view (BamfMatcher *self, BamfView *view)
57+bamf_matcher_register_view_stealing_ref (BamfMatcher *self, BamfView *view)
58 {
59 const char *path, *type;
60 GDBusConnection *connection;
61@@ -248,15 +248,14 @@
62 g_signal_connect (G_OBJECT (view), "active-changed",
63 (GCallback) on_view_active_changed, self);
64
65-
66 if (BAMF_IS_APPLICATION (view))
67 {
68 bamf_matcher_prepare_path_change (self,
69 bamf_application_get_desktop_file (BAMF_APPLICATION (view)), VIEW_ADDED);
70 }
71
72+ // This steals the reference of the view
73 self->priv->views = g_list_prepend (self->priv->views, view);
74- g_object_ref (view);
75
76 g_signal_emit_by_name (self, "view-opened", path, type);
77
78@@ -266,7 +265,7 @@
79 }
80
81 static void
82-bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view, gboolean unref)
83+bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view)
84 {
85 const char * path;
86 const char * type;
87@@ -279,17 +278,23 @@
88 g_signal_handlers_disconnect_by_func (G_OBJECT (view), on_view_closed, self);
89 g_signal_handlers_disconnect_by_func (G_OBJECT (view), on_view_active_changed, self);
90
91- if (unref)
92- {
93- self->priv->views = g_list_remove (self->priv->views, view);
94-
95- if (BAMF_IS_APPLICATION (view))
96- {
97- bamf_matcher_prepare_path_change (self,
98- bamf_application_get_desktop_file (BAMF_APPLICATION (view)),
99- VIEW_REMOVED);
100- }
101-
102+ if (BAMF_IS_APPLICATION (view))
103+ {
104+ bamf_matcher_prepare_path_change (self,
105+ bamf_application_get_desktop_file (BAMF_APPLICATION (view)),
106+ VIEW_REMOVED);
107+ }
108+
109+ if (self->priv->active_app == view)
110+ self->priv->active_app = NULL;
111+
112+ if (self->priv->active_win == view)
113+ self->priv->active_win = NULL;
114+
115+ GList *listed_view = g_list_find (self->priv->views, view);
116+ if (listed_view)
117+ {
118+ self->priv->views = g_list_delete_link (self->priv->views, listed_view);
119 g_object_unref (view);
120 }
121 }
122@@ -789,17 +794,23 @@
123 desktop_file = g_desktop_app_info_new_from_filename (file);
124
125 if (!G_IS_APP_INFO (desktop_file))
126- return;
127+ {
128+ return;
129+ }
130
131 if (!g_desktop_app_info_get_show_in (desktop_file, g_getenv ("XDG_CURRENT_DESKTOP")))
132- return;
133+ {
134+ g_object_unref (desktop_file);
135+ return;
136+ }
137
138 exec = g_strdup (g_app_info_get_commandline (G_APP_INFO (desktop_file)));
139-
140+
141 if (!exec)
142- return;
143-
144- g_object_unref (desktop_file);
145+ {
146+ g_object_unref (desktop_file);
147+ return;
148+ }
149
150 if (exec_string_should_be_processed (self, exec))
151 {
152@@ -825,6 +836,7 @@
153
154 g_free (exec);
155 g_string_free (desktop_id, TRUE);
156+ g_object_unref (desktop_file);
157 }
158
159 static void
160@@ -901,21 +913,21 @@
161 directory = g_path_get_dirname (index_file);
162 input = g_data_input_stream_new (G_INPUT_STREAM (stream));
163
164-
165- while ((line = g_data_input_stream_read_line (input, &length, NULL, NULL)) != NULL)
166+ while ((line = g_data_input_stream_read_line (input, &length, NULL, NULL)))
167 {
168 char *exec;
169 char *filename;
170 GString *desktop_id;
171
172 gchar **parts = g_strsplit (line, "\t", 3);
173-
174 exec = parts[1];
175
176 if (exec_string_should_be_processed (self, exec))
177 {
178 char *tmp = trim_exec_string (self, exec);
179- exec = tmp;
180+ g_free (parts[1]);
181+ parts[1] = tmp;
182+ exec = parts[1];
183 }
184
185 filename = g_build_filename (directory, parts[0], NULL);
186@@ -927,6 +939,7 @@
187 insert_desktop_file_class_into_table (self, filename, desktop_class_table);
188
189 g_string_free (desktop_id, TRUE);
190+ g_free (line);
191 g_free (filename);
192 g_strfreev (parts);
193 length = 0;
194@@ -970,19 +983,22 @@
195 if (!enumerator)
196 continue;
197
198+
199 info = g_file_enumerator_next_file (enumerator, NULL, NULL);
200- for (; info; info = g_file_enumerator_next_file (enumerator, NULL, NULL))
201+
202+ while (info)
203 {
204- if (g_file_info_get_file_type (info) != G_FILE_TYPE_DIRECTORY)
205- continue;
206-
207- subpath = g_build_filename (path, g_file_info_get_name (info), NULL);
208- /* append after the current list item for non-recursive recursion love
209- * and to keep the priorities (hierarchy) of the .desktop directories.
210- */
211- dirs = g_list_insert_before (dirs, l->next, subpath);
212+ if (g_file_info_get_file_type (info) == G_FILE_TYPE_DIRECTORY)
213+ {
214+ /* append after the current list item for non-recursive recursion love
215+ * and to keep the priorities (hierarchy) of the .desktop directories.
216+ */
217+ subpath = g_build_filename (path, g_file_info_get_name (info), NULL);
218+ dirs = g_list_insert_before (dirs, l->next, subpath);
219+ }
220
221 g_object_unref (info);
222+ info = g_file_enumerator_next_file (enumerator, NULL, NULL);
223 }
224
225 g_object_unref (enumerator);
226@@ -1530,10 +1546,17 @@
227 hint = get_window_hint (window, _NET_WM_DESKTOP_FILE);
228 const char *window_class = bamf_legacy_window_get_class_name (window);
229
230- if (hint && hint[0] != '\0' && !is_web_app_window(self, window))
231+ if (hint)
232 {
233- desktop_files = g_list_prepend (desktop_files, hint);
234 /* whew, hard work, didn't even have to make a copy! */
235+ if (hint[0] != '\0' && !is_web_app_window(self, window))
236+ {
237+ desktop_files = g_list_prepend (desktop_files, hint);
238+ }
239+ else
240+ {
241+ g_free (hint);
242+ }
243 }
244 else
245 {
246@@ -1543,7 +1566,7 @@
247
248 if (window_class)
249 {
250- char *window_class_down = g_ascii_strdown (g_strdup(window_class), -1);
251+ char *window_class_down = g_ascii_strdown (window_class, -1);
252 l = g_hash_table_lookup (priv->desktop_id_table, window_class_down);
253 g_free (window_class_down);
254
255@@ -1721,8 +1744,7 @@
256
257 bamf_application_set_wmclass (best, win_class);
258
259- bamf_matcher_register_view (self, BAMF_VIEW (best));
260- g_object_unref (best);
261+ bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (best));
262 }
263
264 g_list_free_full (possible_apps, g_free);
265@@ -1829,8 +1851,7 @@
266 */
267
268 bamfwindow = bamf_window_new (window);
269- bamf_matcher_register_view (self, BAMF_VIEW (bamfwindow));
270- g_object_unref (bamfwindow);
271+ bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (bamfwindow));
272
273 bamf_matcher_setup_window_state (self, bamfwindow);
274 }
275@@ -1892,6 +1913,7 @@
276 }
277
278 g_free (exec);
279+ g_free (role);
280
281 return (list ? (char *) list->data : NULL);
282 }
283@@ -1931,12 +1953,9 @@
284
285 if (bamf_legacy_window_get_window_type (window) == BAMF_WINDOW_DESKTOP)
286 {
287- BamfWindow *bamfwindow;
288-
289- bamfwindow = bamf_window_new (window);
290- bamf_matcher_register_view (self, BAMF_VIEW (bamfwindow));
291- g_object_unref (bamfwindow);
292-
293+ BamfWindow *bamfwindow = bamf_window_new (window);
294+ bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (bamfwindow));
295+
296 return;
297 }
298
299@@ -2045,8 +2064,7 @@
300 if (desktop_file)
301 {
302 best = bamf_application_new_from_desktop_file (desktop_file);
303- bamf_matcher_register_view (self, BAMF_VIEW (best));
304- g_object_unref (best);
305+ bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (best));
306 }
307 }
308
309@@ -2062,7 +2080,7 @@
310 g_return_if_fail (BAMF_IS_MATCHER (self));
311 g_return_if_fail (BAMF_IS_INDICATOR (indicator));
312
313- bamf_matcher_register_view (self, BAMF_VIEW (indicator));
314+ bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (indicator));
315 bamf_matcher_setup_indicator_state (self, indicator);
316 }
317
318@@ -2804,14 +2822,11 @@
319 {
320 BamfMatcher *self = (BamfMatcher *) object;
321 BamfMatcherPrivate *priv = self->priv;
322- GList *l;
323
324- for (l = priv->views; l; l = l->next)
325+ while (priv->views)
326 {
327- bamf_matcher_unregister_view (self, (BamfView*)l->data, FALSE);
328+ bamf_matcher_unregister_view (self, priv->views->data);
329 }
330- g_list_free_full (priv->views, g_object_unref);
331- priv->views = NULL;
332
333 G_OBJECT_CLASS (bamf_matcher_parent_class)->dispose (object);
334 }
335@@ -2875,6 +2890,8 @@
336 priv->active_app = NULL;
337 priv->active_win = NULL;
338
339+ static_matcher = NULL;
340+
341 G_OBJECT_CLASS (bamf_matcher_parent_class)->finalize (object);
342 }
343
344@@ -2899,12 +2916,10 @@
345 BamfMatcher *
346 bamf_matcher_get_default (void)
347 {
348- static BamfMatcher *matcher;
349-
350- if (!BAMF_IS_MATCHER (matcher))
351+ if (!BAMF_IS_MATCHER (static_matcher))
352 {
353- matcher = (BamfMatcher *) g_object_new (BAMF_TYPE_MATCHER, NULL);
354+ static_matcher = g_object_new (BAMF_TYPE_MATCHER, NULL);
355 }
356
357- return matcher;
358+ return static_matcher;
359 }
360
361=== modified file 'src/bamf-view.c'
362--- src/bamf-view.c 2012-01-03 16:23:18 +0000
363+++ src/bamf-view.c 2012-03-19 17:14:17 +0000
364@@ -388,7 +388,7 @@
365 bamf_view_user_visible_changed (view, user_visible);
366 }
367
368-char *
369+const char *
370 bamf_view_get_icon (BamfView *view)
371 {
372 g_return_val_if_fail (BAMF_IS_VIEW (view), NULL);
373@@ -399,12 +399,15 @@
374 return NULL;
375 }
376
377-char *
378+const char *
379 bamf_view_get_name (BamfView *view)
380 {
381 g_return_val_if_fail (BAMF_IS_VIEW (view), NULL);
382
383- return g_strdup (view->priv->name);
384+ if (BAMF_VIEW_GET_CLASS (view)->get_name)
385+ return BAMF_VIEW_GET_CLASS (view)->get_name (view);
386+
387+ return view->priv->name;
388 }
389
390 void
391@@ -585,10 +588,9 @@
392 GDBusMethodInvocation *invocation,
393 BamfView *view)
394 {
395- char *icon = bamf_view_get_icon (view);
396+ const char *icon = bamf_view_get_icon (view);
397 g_dbus_method_invocation_return_value (invocation,
398 g_variant_new ("(s)", icon ? icon : ""));
399- g_free (icon);
400
401 return TRUE;
402 }
403@@ -598,10 +600,9 @@
404 GDBusMethodInvocation *invocation,
405 BamfView *view)
406 {
407- char *name = bamf_view_get_name (view);
408+ const char *name = bamf_view_get_name (view);
409 g_dbus_method_invocation_return_value (invocation,
410 g_variant_new ("(s)", name ? name : ""));
411- g_free (name);
412
413 return TRUE;
414 }
415
416=== modified file 'src/bamf-view.h'
417--- src/bamf-view.h 2012-01-03 16:23:18 +0000
418+++ src/bamf-view.h 2012-03-19 17:14:17 +0000
419@@ -42,9 +42,10 @@
420 GList *names;
421
422 /*< methods >*/
423- const char * (*view_type) (BamfView *view);
424- char * (*stable_bus_name) (BamfView *view);
425- char * (*get_icon) (BamfView *view);
426+ const char * (*view_type) (BamfView *view);
427+ char * (*stable_bus_name) (BamfView *view);
428+ const char * (*get_name) (BamfView *view);
429+ const char * (*get_icon) (BamfView *view);
430
431 /*< random stuff >*/
432 gboolean (* urgent_changed) (BamfView *view, gboolean urgent);
433@@ -97,12 +98,12 @@
434 gboolean bamf_view_is_urgent (BamfView *view);
435 void bamf_view_set_urgent (BamfView *view, gboolean urgent);
436
437-char * bamf_view_get_icon (BamfView *view);
438+const char * bamf_view_get_icon (BamfView *view);
439
440-char * bamf_view_get_name (BamfView *view);
441+const char * bamf_view_get_name (BamfView *view);
442 void bamf_view_set_name (BamfView *view, const char * name);
443
444-char * bamf_view_get_parent_path (BamfView *view);
445+const char * bamf_view_get_parent_path (BamfView *view);
446
447 BamfView * bamf_view_get_parent (BamfView *view);
448 void bamf_view_set_parent (BamfView *view, BamfView *parent);

Subscribers

People subscribed via source and target branches