Re: [PATCH] More valgrind memory leaks
Hi,
* Kyle Wheeler [06-07-08 11:53:46 -0400] wrote:
On Saturday, July 8 at 09:44 AM, quoth Rocco Rutte:
On first sight, you're fix really leaks memory since the IMAP_DATA::cmdbuf member is a BUFFER*, not char*. So at least you
should use mutt_buffer_free() on it, not FREE().
Ahh, good point. That’s what makes the other leak show up after I “fixed” this
one. Using mutt_buffer_free fixes both.
Okay, so I'll replace the call and try here.
However, I think we can ignore this one. Your patch adds the FREE() call to imap_logout() which is only used in imap_logout_all()
which is only used in main.c right before exiting. The OS will mark any memory of the process as available after its exit
regardless of whether it was leaked or not, so...
I think it's a good practice to, when mutt is compiled as a debug version, totally clean up all of its memory so real leaks can
be more easily identified.
In other words, it’s unnecessary but useful for development?
In my opinion: yes. Maybe we could add a special configure option to
enable cleanup code and try to run that version from time to time under
valgrind? That way we could make sure that everything is okay.
For example, I get a ~470 KByte leak valgrind calls 'possibly lost' for
all regex options... and from just valgrind output it's hard to tell
whether that's just not cleaned up memory after exit or if there's a
real leak inside somewhere... And almost 0.5 MByte matter, of course.
Anyway, with those two “leaks” fixed, the only leaks that remain are all the ones in the SASL/SSL libraries, a getpwuid one
that I don’t think we can fix, and the mutt_add_list_n leak that you found:
(it's makes me shiver that there can be memleaks in getpwuid_r())
http://salinan.memoryhole.net/~kyle/valgrind1.txt
Any thoughts on how to fix the SASL/SSL ones (or heck, any idea why
mutt_sasl_client_new leaks twice?)?
I don't know SASL well enough: but maybe they are within it, i.e. mutt
does nothing wrong? It could also come from the same direction as the
mutt_add_list_n() one where the storage of the pointers gets overwritten
because of a not well done cleanup or something like that. As this is
all IMAP data it would be enough to zero out some IMAP_DATA to make
valgrind report such lines...
Leaking twice can happen if sasl_client_new() calls malloc() twice. :)
At least the calls are made at different source lines in
sasl_client_new() which you can see from their addresses.
bye, Rocco
--
:wq!