Patch: memoryleak fix in CamelOperation. Was: [Evolution-hackers] Possible memory leak

[Philip Van Hoof <spam pvanhoof be>]

Please review

http://bugzilla.gnome.org/show_bug.cgi?id=509370

On Mon, 2008-01-14 at 14:33 +0100, Philip Van Hoof wrote:
> I think I've found the leak ...
> 
> g_slist_remove_link:
> --------------------
> Removes an element from a GSList, without freeing the element. The
> removed element's next link is set to NULL, so that it becomes a
> self-contained list with one element.
> 
> Notice the "without freeing the element"
> 
> Yet 
> 
> void
> camel_operation_end (CamelOperation *cc)
> {
> 	...
> 
> 	g_free(s);
> 	cc->status_stack = g_slist_remove_link(cc->status_stack,
> 		 cc->status_stack);
> 	UNLOCK();
> 
> 	if (msg) {
> 		cc->status(cc, msg, sofar, oftotal, cc->status_data);
> 		g_free(msg);
> 	}
> }
> 
> I think this needs to be something like this:
> 
> GSList *item = cc->status_stack;
> cc->status_stack = g_slist_remove_link(cc->status_stack, item);
> g_slist_free (item);
> 
> 
> Can somebody with GSList know-how acknowledge this?
> 
> 
> 
> On Mon, 2008-01-14 at 02:28 +0100, Philip Van Hoof wrote:
> > On Mon, 2008-01-14 at 01:47 +0100, Philip Van Hoof wrote:
> > 
> > > I have this memory analysis tool that I'm willing to believe that tells
> > > me this line in camel-folder.c causes 381 times a leaked allocation of
> > > in total 5.95 kb. (I opened one folder of 1000 items). It seems to be
> > > the g_slice_alloc (which does a malloc since I disabled gslice) of the
> > > g_hash_node_new.
> > 
> > I found it, this was my own mistake in camel-lite's adaptations.
> > 
> > I have found another leak in camel-operation.c:
> > 
> > The cc->status_stack = g_slist_prepend(cc->status_stack, s); in
> > camel_operation_start seems to be leaked each time it's called.
> > 
> > I have 23 calls leaking 184 bytes. Various callers of
> > camel_operation_start (all of them, it seems) seem to create this leak.
> > 
> > My version of camel-operation.c adds two fields (a total and a current
> > position in stead of a percentage, because my requirements where to give
> > accurate to-the-byte progress info, not rounded percentages). Those
> > changes are closely reviewed once more and don't seem to be causing this
> > leak.
> > 
> > ps. I attached the differences that I have. I basically replaced "pc"
> > with a "sofar" and a "oftotal".
> > 
> > 
> > _______________________________________________
> > Evolution-hackers mailing list
> > Evolution-hackers gnome org
> > http://mail.gnome.org/mailman/listinfo/evolution-hackers
-- 
Philip Van Hoof, freelance software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://pvanhoof.be/blog
http://codeminded.be