On Friday, 07 July 2006 at 12:08, Rocco Rutte wrote: > Hi, > > * Rocco Rutte [06-07-07 09:25:24 +0000] wrote: > Now as my tests seem to proof, the leak only happens if a mailbox has > totally new mails. It could be nice if someone could verify that with > valgrind. Also, as the leaked memory is quite small per mailbox, I think > this proofs my assumption that the loop is supposed to abort for the > first totally new message. > > And that is where I guess the leak is: even for such a message, memory > is allocated but obviously never used since there's nothing to fetch > from the hcache. > > Attached are two patches. > > The one with dprint() calls prints the code path inside > imap_read_headers(). The debug file shows that for the first totally new > mail, no "STEP" debug line will be printed but one line per known > message, _and_ the loop is stopped. But since we allocated memory, the > break statement is IMHO exactly where the leak is. It would be nice if > someone could try the patch-mem-verbose.diff patch with and without the > imap_free_header_data() call before printing the "STEP" debug line. With > the free call, my valgrind is happy but complains without it. > > In case my analysis is right, the second patch contains just this free > call for inclusion in CVS. :) Thanks for walking through that crazy code path. Reading your patch, your analysis makes perfect sense to me, and I've added a couple more frees in error conditions. > And while I'm running valgrind, now I got another one: > > ==38018== 69 bytes in 5 blocks are definitely lost in loss record 9 of 57 > ==38018== at 0x3C03A19B: malloc (in > /usr/local/lib/valgrind/vgpreload_memcheck.so) > ==38018== by 0x80A4072: safe_malloc (lib.c:146) > ==38018== by 0x80A50B1: mutt_add_list_n (muttlib.c:246) > ==38018== by 0x80C19A4: imap_open_mailbox (imap.c:616) > ==38018== by 0x808104B: mx_open_mailbox (mx.c:718) > ==38018== by 0x80787AE: main (main.c:974) > > where muttlib.c:246 is: > > head = tmp = safe_malloc (sizeof (LIST)); > > This is supposed to create the list if the given head is NULL. I don't > know in which direction these assignments are resolved, but since: > > head = (tmp = ...); > > and: > > tmp = safe_malloc(); head = tmp; > > still show a leak, I guess the code is fine (i.e. right to left > resolution) and the problem is somewhere else. But the handling of > idata->mboxcache looks okay to me, so I can't much here (yet). Ok. I don't see it at the moment either.
Attachment:
pgpB6BmekLZ6E.pgp
Description: PGP signature