Re: Patches

[David Champion <dgc@xxxxxxxxxxxx>]

I guess I wasn't fully awake yet when I wrote before.  Thomas's question
provokes some deeper consideration. :)


* On 2006.04.28, in <20060428141004.GC32604@xxxxxxxxxxxxxxxxxxxxxxxxxx>,
*       "Thomas Roessler" <roessler@xxxxxxxxxxxxxxxxxx> wrote:
> 
> so, if I get the descriptions of what happens here right, it
> would be enough to have the index code cache that it knows
> about attachment counts, instead of either re-parsing MIME
> parts, or keeping trees around?

That's correct... the catch is that the code already does cache
attachment counts.  Here's mutt_count_body_parts() from CVS:

int mutt_count_body_parts (HEADER *hdr, int flags)
{
  if (hdr->attach_valid && !(flags & M_PARTS_RECOUNT))
    return hdr->attach_total;

  if (AttachAllow || AttachExclude || InlineAllow || InlineExclude)
    hdr->attach_total = count_body_parts(hdr->content, flags | 
M_PARTS_TOPLEVEL);
  else
    hdr->attach_total = 0;

  hdr->attach_valid = 1;
  return hdr->attach_total;
}

Here's the patched code in hdrline.c:

-        if (hdr->content->parts)
-          count = mutt_count_body_parts(hdr, 0);
-        else
-        {
-         mutt_parse_mime_message(ctx, hdr);
-          count = mutt_count_body_parts(hdr, 0);
-         mutt_free_body(&hdr->content->parts);
-        }
-
+       if (!hdr->content->parts)
+         mutt_parse_mime_message (ctx, hdr);
+       count = mutt_count_body_parts (hdr, 0);
+       

So any time mutt_count_body_parts() is called, it returns a cached
count immediately unless the cache value does not exist or has been
invalidated, or the M_PARTS_RECOUNT flag is set in the second parameter
of mutt_count_body_parts().  The flag is clearly not set, so either
this problem is not currently occurring, or something is unmarking
hdr->attach_valid.

hdr->attach_valid is only cleared when muttrc processes "attach" or
"unattach" directions, so if the problem occurs in current CVS, it seems
that something must be reprocessing attach/unattach lines.

I think the current design is sound, unless there's a bug I've
overlooked, but I'm not sure why there appears to be a problem.

Rocco: are you certain that this is a problem in current cvs?  The
caching code is newer than the original attachments patch, and the
patch went into cvs for a day or two before the counting was added,
so it's possible that it was a problem for a while but no longer.

Otherwise, we should figure out why mutt_count_body_parts() is falling
through the cache check and fix that instead.

... Actually, one other answer occurs to me.  Is there a path in the
IMAP code (I'm not that familiar with it) whereby a message's whole
HEADER* is tossed and replaced?

-- 
 -D.    dgc@xxxxxxxxxxxx        NSIT    University of Chicago