From: Kevin McCarthy Date: Sun, 21 May 2017 01:52:18 +0000 (-0700) Subject: Add msn_index and max_msn to find and check boundaries by MSN. (see #3942) X-Git-Tag: neomutt-20170707^2^2~41 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bb5fb98a14f505468d04086a7ace80b8dac10e61;p=neomutt Add msn_index and max_msn to find and check boundaries by MSN. (see #3942) Since there can be gaps in MSNs, the largest MSN in the context is not necessarily ctx->msgcount. Use max_msn instead of ctx->msgcount for: - the starting MSN of new mail header fetching - boundary checking in fetch, expunge, and other places Use msn_index to efficiently look up headers by MSN. This makes the expunge code slightly more efficient. It also makes FETCH handling, and duplicate FETCH FLAG handling efficient. --- diff --git a/imap/command.c b/imap/command.c index 7230da3d9..87802dcda 100644 --- a/imap/command.c +++ b/imap/command.c @@ -279,22 +279,18 @@ void imap_cmd_finish (IMAP_DATA* idata) if (idata->reopen & IMAP_REOPEN_ALLOW) { - int count = idata->newMailCount; + unsigned int count = idata->newMailCount; if (!(idata->reopen & IMAP_EXPUNGE_PENDING) && (idata->reopen & IMAP_NEWMAIL_PENDING) - && count > idata->ctx->msgcount) + && count > idata->max_msn) { /* read new mail messages */ dprint (2, (debugfile, "imap_cmd_finish: Fetching new mail\n")); /* check_status: curs_main uses imap_check_mailbox to detect * whether the index needs updating */ idata->check_status = IMAP_NEWMAIL_PENDING; - /* TODO: - * The second parameter is wrong. We need to start from - * idata->max_msn + 1. - */ - imap_read_headers (idata, idata->ctx->msgcount+1, count); + imap_read_headers (idata, idata->max_msn+1, count); } else if (idata->reopen & IMAP_EXPUNGE_PENDING) { @@ -462,7 +458,7 @@ static int cmd_handle_untagged (IMAP_DATA* idata) { char* s; char* pn; - int count; + unsigned int count; s = imap_next_word (idata->buf); pn = imap_next_word (s); @@ -483,7 +479,7 @@ static int cmd_handle_untagged (IMAP_DATA* idata) count = atoi (pn); if ( !(idata->reopen & IMAP_EXPUNGE_PENDING) && - count < idata->ctx->msgcount) + count < idata->max_msn) { /* Notes 6.0.3 has a tendency to report fewer messages exist than * it should. */ @@ -492,7 +488,7 @@ static int cmd_handle_untagged (IMAP_DATA* idata) } /* at least the InterChange server sends EXISTS messages freely, * even when there is no new mail */ - else if (count == idata->ctx->msgcount) + else if (count == idata->max_msn) dprint (3, (debugfile, "cmd_handle_untagged: superfluous EXISTS message.\n")); else @@ -593,34 +589,37 @@ static void cmd_parse_capability (IMAP_DATA* idata, char* s) * be reopened at our earliest convenience */ static void cmd_parse_expunge (IMAP_DATA* idata, const char* s) { - unsigned int exp_msn; - int cur; + unsigned int exp_msn, cur; HEADER* h; dprint (2, (debugfile, "Handling EXPUNGE\n")); exp_msn = atoi (s); + if (exp_msn < 1 || exp_msn > idata->max_msn) + return; - /* walk headers, zero seqno of expunged message, decrement seqno of those - * above. Possibly we could avoid walking the whole list by resorting - * and guessing a good starting point, but I'm guessing the resort would - * nullify the gains */ - for (cur = 0; cur < idata->ctx->msgcount; cur++) + h = idata->msn_index[exp_msn - 1]; + if (h) { - h = idata->ctx->hdrs[cur]; + /* imap_expunge_mailbox() will rewrite h->index. + * It needs to resort using SORT_ORDER anyway, so setting to INT_MAX + * makes the code simpler and possibly more efficient. */ + h->index = INT_MAX; + HEADER_DATA(h)->msn = 0; + } - if (HEADER_DATA(h)->msn == exp_msn) - { - /* imap_expunge_mailbox() will rewrite h->index. - * It needs to resort using SORT_ORDER anyway, so setting to INT_MAX - * makes the code simpler and possibly more efficient. */ - h->index = INT_MAX; - HEADER_DATA(h)->msn = 0; - } - else if (HEADER_DATA(h)->msn > exp_msn) + /* decrement seqno of those above. */ + for (cur = exp_msn; cur < idata->max_msn; cur++) + { + h = idata->msn_index[cur]; + if (h) HEADER_DATA(h)->msn--; + idata->msn_index[cur - 1] = h; } + idata->msn_index[idata->max_msn - 1] = NULL; + idata->max_msn--; + idata->reopen |= IMAP_EXPUNGE_PENDING; } @@ -631,35 +630,25 @@ static void cmd_parse_expunge (IMAP_DATA* idata, const char* s) static void cmd_parse_fetch (IMAP_DATA* idata, char* s) { unsigned int msn; - int cur; - HEADER* h = NULL; + HEADER *h; dprint (3, (debugfile, "Handling FETCH\n")); msn = atoi (s); + if (msn < 1 || msn > idata->max_msn) + { + dprint (3, (debugfile, "FETCH response ignored for this message\n")); + return; + } - /* TODO: this is wrong. Should compare to idata->max_msn, to be added. */ - if (msn <= idata->ctx->msgcount) - /* see cmd_parse_expunge */ - for (cur = 0; cur < idata->ctx->msgcount; cur++) - { - h = idata->ctx->hdrs[cur]; - - if (h && h->active && HEADER_DATA(h)->msn == msn) - { - dprint (2, (debugfile, "Message UID %d updated\n", HEADER_DATA(h)->uid)); - break; - } - - h = NULL; - } - - if (!h) + h = idata->msn_index[msn - 1]; + if (!h || !h->active) { dprint (3, (debugfile, "FETCH response ignored for this message\n")); return; } - + + dprint (2, (debugfile, "Message UID %d updated\n", HEADER_DATA(h)->uid)); /* skip FETCH */ s = imap_next_word (s); s = imap_next_word (s); diff --git a/imap/imap.c b/imap/imap.c index 2e27e1c54..5d6f6a2b3 100644 --- a/imap/imap.c +++ b/imap/imap.c @@ -608,6 +608,7 @@ static int imap_open_mailbox (CONTEXT* ctx) idata->status = 0; memset (idata->ctx->rights, 0, sizeof (idata->ctx->rights)); idata->newMailCount = 0; + idata->max_msn = 0; mutt_message (_("Selecting %s..."), idata->mailbox); imap_munge_mbox_name (idata, buf, sizeof(buf), idata->mailbox); @@ -1406,6 +1407,8 @@ int imap_close_mailbox (CONTEXT* ctx) if (ctx->hdrs[i] && ctx->hdrs[i]->data) imap_free_header_data ((IMAP_HEADER_DATA**)&(ctx->hdrs[i]->data)); hash_destroy (&idata->uid_hash, NULL); + FREE (&idata->msn_index); + idata->msn_index_size = 0; for (i = 0; i < IMAP_CACHE_LEN; i++) { diff --git a/imap/imap_private.h b/imap/imap_private.h index 67f4dd7ef..ff28da2f1 100644 --- a/imap/imap_private.h +++ b/imap/imap_private.h @@ -212,11 +212,14 @@ typedef struct char *mailbox; unsigned short check_status; unsigned char reopen; - unsigned int newMailCount; + unsigned int newMailCount; /* Set when EXISTS notifies of new mail */ IMAP_CACHE cache[IMAP_CACHE_LEN]; HASH *uid_hash; unsigned int uid_validity; unsigned int uidnext; + HEADER **msn_index; /* look up headers by (MSN-1) */ + unsigned int msn_index_size; /* allocation size */ + unsigned int max_msn; /* the largest MSN fetched so far */ body_cache_t *bcache; /* all folder flags - system flags AND keywords */ @@ -264,7 +267,7 @@ int imap_cmd_idle (IMAP_DATA* idata); /* message.c */ void imap_add_keywords (char* s, HEADER* keywords, LIST* mailbox_flags, size_t slen); void imap_free_header_data (IMAP_HEADER_DATA** data); -int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end); +int imap_read_headers (IMAP_DATA* idata, unsigned int msn_begin, unsigned int msn_end); char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s); int imap_cache_del (IMAP_DATA* idata, HEADER* h); int imap_cache_clean (IMAP_DATA* idata); diff --git a/imap/message.c b/imap/message.c index 36758cb94..85d8f408e 100644 --- a/imap/message.c +++ b/imap/message.c @@ -69,12 +69,34 @@ static void imap_update_context (IMAP_DATA *idata, int oldmsgcount) } } +static void imap_alloc_msn_index (IMAP_DATA *idata, unsigned int msn_count) +{ + unsigned int new_size; + + if (msn_count <= idata->msn_index_size) + return; + + /* Add a little padding, like mx_allloc_memory() */ + new_size = msn_count + 25; + + if (!idata->msn_index) + idata->msn_index = safe_calloc (new_size, sizeof (HEADER *)); + else + { + safe_realloc (&idata->msn_index, sizeof (HEADER *) * new_size); + memset (idata->msn_index + idata->msn_index_size, 0, + sizeof (HEADER *) * (new_size - idata->msn_index_size)); + } + + idata->msn_index_size = new_size; +} + /* imap_read_headers: * Changed to read many headers instead of just one. It will return the * msn of the last message read. It will return a value other than * msn_end if mail comes in while downloading headers (in theory). */ -int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) +int imap_read_headers (IMAP_DATA* idata, unsigned int msn_begin, unsigned int msn_end) { CONTEXT* ctx; char *hdrreq = NULL; @@ -131,6 +153,7 @@ int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) /* make sure context has room to hold the mailbox */ while (msn_end > ctx->hdrmax) mx_alloc_memory (ctx); + imap_alloc_msn_index (idata, msn_end); idx = ctx->msgcount; oldmsgcount = ctx->msgcount; @@ -190,10 +213,24 @@ int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) if (!h.data->uid) { dprint (2, (debugfile, "imap_read_headers: skipping hcache FETCH " + "response for message number %d missing a UID\n", h.data->msn)); + continue; + } + + if (h.data->msn < 1 || h.data->msn > msn_end) + { + dprint (1, (debugfile, "imap_read_headers: skipping hcache FETCH " "response for unknown message number %d\n", h.data->msn)); continue; } + if (idata->msn_index[h.data->msn - 1]) + { + dprint (2, (debugfile, "imap_read_headers: skipping hcache FETCH " + "for duplicate message %d\n", h.data->msn)); + continue; + } + ctx->hdrs[idx] = imap_hcache_get (idata, h.data->uid); if (ctx->hdrs[idx]) { @@ -202,6 +239,9 @@ int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) * assumption. */ msn_begin = MAX (msn_begin, h.data->msn + 1); + idata->max_msn = MAX (idata->max_msn, h.data->msn); + idata->msn_index[h.data->msn - 1] = ctx->hdrs[idx]; + ctx->hdrs[idx]->index = idx; /* messages which have not been expunged are ACTIVE (borrowed from mh * folders) */ @@ -293,21 +333,18 @@ int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) } /* May receive FLAGS updates in a separate untagged response (#2935) */ - /* - * TODO: This check was introduced in e5c2befbf0f5 - * But was broken in 0e4f1782ea2e - * We need to check against idata->msn_index to see if - * we already have a header downloaded, when that is introduced. - if (idx < ctx->msgcount) + if (idata->msn_index[h.data->msn - 1]) { - dprint (2, (debugfile, "imap_read_headers: message %d is not new\n", - h.data->msn)); + dprint (2, (debugfile, "imap_read_headers: skipping FETCH response for " + "duplicate message %d\n", h.data->msn)); continue; } - */ ctx->hdrs[idx] = mutt_new_header (); + idata->max_msn = MAX (idata->max_msn, h.data->msn); + idata->msn_index[h.data->msn - 1] = ctx->hdrs[idx]; + ctx->hdrs[idx]->index = idx; /* messages which have not been expunged are ACTIVE (borrowed from mh * folders) */ @@ -362,6 +399,7 @@ int imap_read_headers (IMAP_DATA* idata, int msn_begin, int msn_end) msn_end = idata->newMailCount; while (msn_end > ctx->hdrmax) mx_alloc_memory (ctx); + imap_alloc_msn_index (idata, msn_end); idata->reopen &= ~IMAP_NEWMAIL_PENDING; idata->newMailCount = 0; }