Re: [fetchmail-devel] fetchmail fix coordination and future planning

[Matthias A. <mat...@gm...>]

Am 03.05.2011 20:25, schrieb Sunil Shetye:
> Hi Matthias,
> 
> 
> 
> ----- Original Message -----
>> As so the second (with _split and caching), I wonder if it's the right
>> approach. It looks a bit as though we're curing the symptoms instead of
>> the cause.  The cause is a buffer too small to handle the response, and
>> the cure would be to make the buffer dynamic and possibly resize it if
>> we don't get a \r?\n in it.
>>
>>
>> A dynamic buffer could also save us the strcpy(argbuf, buf) copy which,
>> IMO, is quite wasteful.
> 
> I had given a thought for implementing dynamic buffers. However, some issues came to my mind:
> 
> - What if the buffer required is very long? For example, an extreme response like:
> 
> IMAP< * SEARCH 1 2 3 4 5 6 ... 999995 999996 999997 99998 99999 100000
> 
> will require a dynamic buffer of size 575k. Is it okay to allocate such a huge buffer?

Possibly yes, although some limit would be in order.

Do we really need to optimize 6.3.X with obtrusive changes?  This will
make merging between master and next harder.

> - Who will free the buffer? Obviously, it will have to be statically allocated in order to free it later.
> 
> - How long will it remain? Is it safe to free it immediately on the next call?

> In short, without a proper memory manager, it may not be safe to just allocate a dynamic buffer of such a huge size.

I'll sidestep these for now.

> Note that the strcpy(argbuf, buf) can still be avoided by directly passing argbuf (if available) to gen_recv().

OK. It's a separate microoptimization anyways, not worth discussing.
Someone will do it ;-)

> Also, note that that patch is a bug fix. The current method range search for UNSEEN mails is too slow for huge mailboxes. So, a fix for this issue is required for 6.3.X.

It may be painful, but if it works, it's not a bug unless the
documentation promises otherwise (which it does not).

For a 25,000 message mailbox with 1% unseen, the amount of SEARCH
transactions seems to be reasonably low.  I think POP3 + uidl + keep in
that situation would burn something between 3 and 30 seconds CPU time on
a Phenom II 2.5 GHz CPU w/ GCC 4.5 compiled code while chewing on UIDLs.
The fix was contributed by Rainer Weikusat and is in the next branch and
in 7.0.0-alpha2.

>> What I propose is to have three branches, details below the list:
>>
>> - one for 6.3.X (new name to be devised), for a final release and
>> possibly later critical/security fixes
>>
>> - one for a new 6.4.X or 7.X.X branch (master) where such fixes as the
>> IMAP search ranges can be made
> 
> I think, it is necessary to fix the range search issue in 6.3.X itself as fetchmail is now unusable with IMAP when the mailbox is huge and keep is on.

Try POP3 UIDL on a mailbox with the same message (86000) count and let
me know if you still think IMAP is "unusable".  Hint: check what Rainer
and I have been benchmarking when he contributed the PATRICIA
implementation.  I've avoided it in 6.3.X because I have portability
concerns.

I will take your patch in spite of my concerns, and test it.

I suggest that you check the manual pages of strlcpy and strlcat, these
are alternatives to snprintf(buf, siz, "%s%s", s1, s2).  glibc is one of
the major distros lacking it, but we ship replacements.  Also note that
<strings.h> has strcasecmp and, more usefully, strncasecmp, and we have
our replacements for OSes lacking them, too.