GNOME Bugzilla – Bug 304633
fix possible leak in _wnck_get_text_property
Last modified: 2005-05-27 14:35:01 UTC
valgrind once reported this leak : ==6638== 8 bytes in 8 blocks are definitely lost in loss record 111 of 1079 ==6638== at 0x1B904EDD: malloc (vg_replace_malloc.c:131) ==6638== by 0x1C3A4BD6: XGetWindowProperty (in /usr/X11R6/lib/libX11.so.6.2) ==6638== by 0x1C3C26F1: XGetTextProperty (in /usr/X11R6/lib/libX11.so.6.2) ==6638== by 0x1BC2BAEC: _wnck_get_text_property (xutils.c:268)
Created attachment 46593 [details] [review] possible fix i was not able to reproduce the leak, but i hope this patch fix it. I've checked XFree documentation : "An extra byte containing null (which is not included in the nitems member) is stored at the end of the value field of text_prop_return." So nitems may be 0 and value not null.
or may be the test if(text.value) is useless ...
Yeah, Owen warned us about this in bug 133896; we really need someone to audit both libwnck and metacity for XGetWindowProperty calls (though I think Kjartan may have come along and fixed one or two already?).
Okay, so I looked closer and I think your patch is right, but I'm getting a little confused at the documentation (I'm looking at http://tronche.com/gui/x/xlib/ICC/client-to-window-manager/XGetTextProperty.html). So I would like Vincent or Mark or Havoc to take a look as well. (Also, as an aside, I did a grep for all the occurrences of XGetWindowProperty that are called directly from libwnck and they seem to be handled; from your valgrind stack trace it appears that XGetTextProperty calls XGetWindowProperty so I searched for all of those and this appears to be the only one. So I don't think there's other similar leaks to plug).
Comment on attachment 46593 [details] [review] possible fix Thanks!