Compiz Workarounds Plugins

Merge lp:~azzar1/compiz-workarounds-plugin/fix-memory-leak into lp:compiz-workarounds-plugin

  1. fix-memory-leak
  2. Merge into 0.9.5
Proposed by Andrea Azzarone
Status: Merged
Merged at revision: 112
Proposed branch: lp:~azzar1/compiz-workarounds-plugin/fix-memory-leak
Merge into: lp:compiz-workarounds-plugin
Diff against target: 15 lines (+5/-0)
1 file modified
src/workarounds.cpp (+5/-0)
To merge this branch: bzr merge lp:~azzar1/compiz-workarounds-plugin/fix-memory-leak
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
Review via email: mp+69886@code.launchpad.net

Description of the change

Fixes a memory leak. As you can read here (http://tronche.com/gui/x/xlib/ICC/client-to-window-manager/XGetClassHint.html) both res_name and res_class should be freed.

P.S. according to me this 'if' has no sense

if (!XGetClassHint (screen->dpy (), window->id (), &classHint) != Success)

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

+1, merging.

With the XGetClassHint call, it is indeed correct to check for the return value since XGetClassHint is really just a wrapper around XGetWindowProperty which is a synchronous operation, and it is possible that the WM_CLASS property might not be set or the window might have been destroyed, however the way we are doing the check is strange. It should be for if (XGetClassHint ... == Success). I will change that when I merge your branch.

review: Approve
Revision history for this message
Andrea Azzarone (azzar1) wrote :

Sam I was referring just to the double neg.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Ah yes.

It's strange though, looking at that code, since if
(!XGetWindowProperty (..) != Success) cause the function to return if
it succeeds, which isn't want we want. Perhaps it was just a typo when
I ported it :)

On Sun, Jul 31, 2011 at 12:01 AM, Andrea Azzarone <email address hidden> wrote:
> Sam I was referring just to the double neg.
> --
> https://code.launchpad.net/~andyrock/compiz-workarounds-plugin/fix-memory-leak/+merge/69886
> You are reviewing the proposed merge of lp:~andyrock/compiz-workarounds-plugin/fix-memory-leak into lp:compiz-workarounds-plugin.
>

--
Sam Spilsbury

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/workarounds.cpp'
2--- src/workarounds.cpp 2011-05-26 12:52:31 +0000
3+++ src/workarounds.cpp 2011-07-30 13:21:10 +0000
4@@ -719,6 +719,11 @@
5 resName = CompString (classHint.res_name);
6 XFree (classHint.res_name);
7 }
8+
9+ if (classHint.res_class)
10+ {
11+ XFree (classHint.res_class);
12+ }
13
14 /* FIXME: Is this the best way to detect a notification type window? */
15 if (ws->optionGetNotificationDaemonFix ())

Subscribers

People subscribed via source and target branches