Attachment counting feature at risk.
There was some discussion on IRC about this... The conclusion
that it leads me to is that getting attachment counting right
is necessarily much more expensive than the kind of parsing
that mutt currently does in order to get the information it
needs to display a folder's index. Including the cheap variant
(just testing for multiparts) has a ton of weaknesses, but is
the only thing that can be done easily and efficiently.
At this point, I'm quite inclined to drop the current
attachment counting feature from CVS -- what we have now is
general, it gives correct results, and it's awfully
inefficient.
Please scream *now*.
Regards,
--
Thomas Roessler · Personal soap box at <http://log.does-not-exist.org/>.
On 2006-04-28 21:58:48 +0200, Thomas Roessler wrote:
> From: Thomas Roessler <roessler@xxxxxxxxxxxxxxxxxx>
> To: mutt-dev list <mutt-dev@xxxxxxxx>
> Date: Fri, 28 Apr 2006 21:58:48 +0200
> Subject: Re: Patches
> X-Spam-Level:
>
> On 2006-04-28 21:45:45 +0200, Thomas Roessler wrote:
>
> > Oh well. Now trying to hack this into some shape that
> > remotely resembles decent code.
>
> Try the change below the .signature, it's in CVS now. Still,
> playing with this feature a bit, it's a terrible performance
> hog.
>
> If you use this, it quite simply requires mutt to parse every
> individual message when you scroll through an index for the
> first time; with a large-ish mailbox, it's quite painful to
> use, even on modern machines with fast hard disks. It's
> probably ok with a long-running mutt instance that operates,
> say, on an inbox where messages slowly come in and the parsing
> can be done one by one.
>
> I'm wondering, is there anybody here who *seriously* uses this
> feature?
>
> If the main use case is to simply know whether the top-level
> body part is a multipart, then we can get that a *lot* cheaper,
> and with a fraction of the code. Persoally, I don't think that
> this is particularly useful information (and that's why I threw
> the feature out years ago -- it's particularly nonsensical for
> multipart/{signed,encrypted}), but I guess I'd rather include
> that than have the present attachment counting code stay.
>
> Sorry, David...
> --
> Thomas Roessler · Personal soap box at <http://log.does-not-exist.org/>.
>
>
>
>
>
>
>
> Index: hdrline.c
> ===================================================================
> RCS file: /cvs/mutt/mutt/hdrline.c,v
> retrieving revision 3.18
> diff -u -r3.18 hdrline.c
> --- hdrline.c 10 Jan 2006 19:15:21 -0000 3.18
> +++ hdrline.c 28 Apr 2006 19:52:16 -0000
> @@ -657,16 +657,7 @@
>
> case 'X':
> {
> - int count;
> -
> - 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);
> - }
> + int count = mutt_count_body_parts (ctx, hdr);
>
> /* The recursion allows messages without depth to return 0. */
> if (optional)
> Index: mutt.h
> ===================================================================
> RCS file: /cvs/mutt/mutt/mutt.h,v
> retrieving revision 3.64
> diff -u -r3.64 mutt.h
> --- mutt.h 28 Apr 2006 08:35:03 -0000 3.64
> +++ mutt.h 28 Apr 2006 19:52:17 -0000
> @@ -921,9 +921,7 @@
> regex_t minor_rx;
> } ATTACH_MATCH;
>
> -/* Flags for mutt_count_body_parts() */
> #define M_PARTS_TOPLEVEL (1<<0) /* is the top-level part */
> -#define M_PARTS_RECOUNT (1<<1) /* force recount */
>
> #include "ascii.h"
> #include "protos.h"
> Index: parse.c
> ===================================================================
> RCS file: /cvs/mutt/mutt/parse.c,v
> retrieving revision 3.22
> diff -u -r3.22 parse.c
> --- parse.c 16 Dec 2005 18:49:40 -0000 3.22
> +++ parse.c 28 Apr 2006 19:52:18 -0000
> @@ -928,7 +928,6 @@
> void mutt_parse_mime_message (CONTEXT *ctx, HEADER *cur)
> {
> MESSAGE *msg;
> - int flags = 0;
>
> do {
> if (cur->content->type != TYPEMESSAGE &&
> @@ -949,7 +948,7 @@
> }
> } while (0);
>
> - mutt_count_body_parts(cur, flags|M_PARTS_RECOUNT);
> + cur->attach_valid = 0;
> }
>
> int mutt_parse_rfc822_line (ENVELOPE *e, HEADER *hdr, char *line, char *p,
> short user_hdrs, short weed,
> @@ -1599,16 +1598,27 @@
> return count < 0 ? 0 : count;
> }
>
> -int mutt_count_body_parts (HEADER *hdr, int flags)
> +int mutt_count_body_parts (CONTEXT *ctx, HEADER *hdr)
> {
> - if (hdr->attach_valid && !(flags & M_PARTS_RECOUNT))
> - return hdr->attach_total;
> + short keep_parts = 0;
>
> + if (hdr->attach_valid)
> + return hdr->attach_total;
> +
> + if (hdr->content->parts)
> + keep_parts = 1;
> + else
> + mutt_parse_mime_message (ctx, hdr);
> +
> if (AttachAllow || AttachExclude || InlineAllow || InlineExclude)
> - hdr->attach_total = count_body_parts(hdr->content, flags |
> M_PARTS_TOPLEVEL);
> + hdr->attach_total = count_body_parts(hdr->content, M_PARTS_TOPLEVEL);
> else
> hdr->attach_total = 0;
>
> hdr->attach_valid = 1;
> +
> + if (!keep_parts)
> + mutt_free_body (&hdr->content->parts);
> +
> return hdr->attach_total;
> }
> Index: pattern.c
> ===================================================================
> RCS file: /cvs/mutt/mutt/pattern.c,v
> retrieving revision 3.29
> diff -u -r3.29 pattern.c
> --- pattern.c 15 Jan 2006 21:37:03 -0000 3.29
> +++ pattern.c 28 Apr 2006 19:52:18 -0000
> @@ -1132,17 +1132,7 @@
> return (pat->not ^ (h->thread && h->thread->duplicate_thread));
> case M_MIMEATTACH:
> {
> - int count;
> -
> - if (h->content->parts)
> - count = mutt_count_body_parts(h, 0);
> - else
> - {
> - mutt_parse_mime_message(ctx, h);
> - count = mutt_count_body_parts(h, 0);
> - mutt_free_body(&h->content->parts);
> - }
> -
> + int count = mutt_count_body_parts (ctx, h);
> return (pat->not ^ (count >= pat->min && (pat->max == M_MAXRANGE ||
> count <= pat->max)));
> }
> Index: protos.h
> ===================================================================
> RCS file: /cvs/mutt/mutt/protos.h,v
> retrieving revision 3.43
> diff -u -r3.43 protos.h
> --- protos.h 9 Jan 2006 19:43:59 -0000 3.43
> +++ protos.h 28 Apr 2006 19:52:18 -0000
> @@ -168,7 +168,7 @@
> void mutt_buffy (char *, size_t);
> int mutt_buffy_list (void);
> void mutt_canonical_charset (char *, size_t, const char *);
> -int mutt_count_body_parts (HEADER *hdr, int flags);
> +int mutt_count_body_parts (CONTEXT *, HEADER *);
> void mutt_check_rescore (CONTEXT *);
> void mutt_clear_error (void);
> void mutt_create_alias (ENVELOPE *, ADDRESS *);
>
>