Re: [PATCH] More valgrind memory leaks

[Kyle Wheeler <kyle-mutt-dev@xxxxxxxxxxxxxx>]

On Saturday, July  8 at 09:44 AM, quoth Rocco Rutte:
> >   784 (16 direct, 768 indirect) bytes in 1 blocks are definitely       
> >   lost in loss record 217 of 279
> >      at 0x401B422: malloc (vg_replace_malloc.c:149)
> >      by 0x80C2020: safe_malloc (lib.c:146)
> >      by 0x80C6433: mutt_buffer_init (muttlib.c:1388)
> >      by 0x80EABF3: imap_new_idata (util.c:264)
> >      by 0x80E33C3: imap_conn_find (imap.c:350)
> >      by 0x80E3A54: imap_open_mailbox (imap.c:553)
> >      by 0x80928BC: mx_open_mailbox (mx.c:698)
> >      by 0x8087389: main (main.c:963)
>
> That looks like the 2nd one from one of my older runs:
>
>   <http://user.cs.tu-berlin.de/~pdmef/tmp/valgrind.txt>
>
> 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.

> 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?

Well, my patch may be in the wrong place. In general, I think, should 
mutt ever begin to handle broken connections successfully (currently, 
if my IMAP connection dies for any reason, I have to quit/kill mutt 
and start over again, because it hangs in the SSL library on 
reconnection), this will need to be handled in some way.

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:

   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?)?

~Kyle
--
If God can work through me, he can work through anyone.
                                             -- St. Francis of Assisi

Attachment: pgpVDCWp1YD5s.pgp
Description: PGP signature