Re: [fetchmail-devel] [PATCH-3] Re: fetchmail IMAP bug - causes some mails to not be delivered when using IDLE

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

Am 21.01.2010, 12:51 Uhr, schrieb Matt Doran <mat...@pa...>:

> On 19/01/2010 2:13 PM, Matt Doran wrote:
>> Sunil Shetye wrote:
>>> Quoting from Matthias Andree's mail on Thu, Jan 14, 2010:
>>>
>>>> Perhaps a separate imap_untagged_parse() function would be more  
>>>> suitable here,
>>>> and possibly called from inside imap_ok()? See below.
>>>>
>>>
>>> Ok, I have made a separate imap_untagged_response() to parse the
>>> untagged response. Other key changes:
>>>
>>> - use  stage == STAGE_GETAUTH&&  !strncmp(buf, "* CAPABILITY", 12)
>>>    - instead of  strstr(buf, " CAPABILITY")
>>>
>>> - use  stage == STAGE_GETAUTH&&  !strncmp(buf, "* PREAUTH", 9)
>>>    - instead of  strstr(buf, " PREAUTH")
>>>
>>> - use  stage == STAGE_GETRANGE&&  !check_only&&  strstr(buf,  
>>> "[READ-ONLY]")
>>>    - instead of  !check_only&&  strstr(buf, "[READ-ONLY]")
>>>
>>> - log  "* BYE"  message
>>>
>>>
>>> These changes should save a lot of string comparisons during runtime.
>>>
>>>
>>>> Which raises another question: does it make sense to have  
>>>> caller-specific
>>>> parsers for untagged replies? Perhaps those functions that need them  
>>>> - for
>>>> instance for * SEARCH, should call some new imap_ok_with_untagged()  
>>>> function
>>>> instead, and the normal imap_ok handles all untagged implicitly (we  
>>>> can rename
>>>> imap_ok, and uses dummy functions that just pass in an additional  
>>>> argument
>>>> whether or not returning PS_UNTAGGED is requested).
>>>>
>>>
>>> In order to address this, I have made one more intermediate function
>>> imap_response(). Caller-specific parsers will call imap_response() in
>>> a loop and others will continue calling imap_ok().
>>>
>>> On reading the RFCs, it looks like the [READ-ONLY] flag is actually
>>> part of a tagged response. Should the [READ-ONLY] check be moved to
>>> imap_response()?
>>>
>>> Functions cleaned up:
>>>
>>> imap_getrange()
>>> imap_getpartialsizes()
>>> imap_trail()
>>>
>>>
>>>> I think (unless I'm mistaken) we need to deal with asynchronous  
>>>> (non-requested)
>>>> untagged messages anyways, so perhaps all of the untagged logic needs  
>>>> to be in
>>>> imap_ok, but I'm not sure about that.  Since I haven't looked at the  
>>>> imap.c code
>>>> lately, please only consider this brainstorming or "thinking aloud"  
>>>> and not a
>>>> plea or request to do things in a certain way.  Design it the way you  
>>>> think fits
>>>> the needs best.
>>>>
>>>
>>> Please check if this patch addresses all the issues raised.
>>>
>>>
>> OK, tested and it looks good.   See -v -v output attached.
>>
>>
>
> Just letting you know I've been running this for a few days now with no
> problems.

Hi Matt,

thanks for the continued testing of this new feature, and helping us  
proceed with the work.

Dear Sunil,

thanks a lot for your work on this. I hope to audit and possibly commit  
the patch tonight.

Best regards
Matthias

-- 
Matthias Andree