From 48211b98f94e9437643cbda7d4f5e0e1061ad28e Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Sun, 11 Dec 2005 22:51:29 +0000 Subject: [PATCH] IMAP command batching code, used to pipeline mailbox poll requests. Up to 10 poll commands will be sent at a time (tunable in imap_private.h). This is a huge win on my currently awful wireless link. It takes a knife to a lot of fundamental IMAP code (mostly for the better), so it may have destabilised things. Time for some brave (or lazy non-Changelog-reading) testers to report... next up, IDLE support. --- buffy.c | 24 ++----- imap/command.c | 138 ++++++++++++++++++++++++++++++-------- imap/imap.c | 157 +++++++++++++++++++++++++++++++++++--------- imap/imap.h | 2 +- imap/imap_private.h | 9 ++- imap/util.c | 2 +- 6 files changed, 250 insertions(+), 82 deletions(-) diff --git a/buffy.c b/buffy.c index 11d133d8..2e5eea03 100644 --- a/buffy.c +++ b/buffy.c @@ -286,6 +286,8 @@ int mutt_buffy_check (int force) BuffyNotify = 0; #ifdef USE_IMAP + BuffyCount += imap_buffy_check (force); + if (!Context || Context->magic != M_IMAP) #endif #ifdef USE_POP @@ -300,13 +302,10 @@ int mutt_buffy_check (int force) for (tmp = Incoming; tmp; tmp = tmp->next) { +#ifndef USE_IMAP tmp->new = 0; - -#ifdef USE_IMAP - if (mx_is_imap (tmp->path)) - tmp->magic = M_IMAP; - else #endif + #ifdef USE_POP if (mx_is_pop (tmp->path)) tmp->magic = M_POP; @@ -398,21 +397,6 @@ int mutt_buffy_check (int force) if ((tmp->new = mh_buffy (tmp->path)) > 0) BuffyCount++; break; - -#ifdef USE_IMAP - case M_IMAP: - if ((tmp->new = imap_mailbox_check (tmp->path, 1)) > 0) - BuffyCount++; - else - tmp->new = 0; - - break; -#endif - -#ifdef USE_POP - case M_POP: - break; -#endif } } #ifdef BUFFY_SIZE diff --git a/imap/command.c b/imap/command.c index f2986d2f..e163b260 100644 --- a/imap/command.c +++ b/imap/command.c @@ -29,6 +29,7 @@ #include "imap_private.h" #include "message.h" #include "mx.h" +#include "buffy.h" #include #include @@ -46,6 +47,7 @@ static void cmd_parse_lsub (IMAP_DATA* idata, char* s); static void cmd_parse_fetch (IMAP_DATA* idata, char* s); static void cmd_parse_myrights (IMAP_DATA* idata, const char* s); static void cmd_parse_search (IMAP_DATA* idata, const char* s); +static void cmd_parse_status (IMAP_DATA* idata, char* s); static char *Capabilities[] = { "IMAP4", @@ -62,15 +64,11 @@ static char *Capabilities[] = { NULL }; -/* imap_cmd_start: Given an IMAP command, send it to the server. - * Currently a minor convenience, but helps to route all IMAP commands - * through a single interface. */ -int imap_cmd_start (IMAP_DATA* idata, const char* cmdstr) +/* imap_cmd_queue: Add command to command queue. Fails if the queue is full. */ +int imap_cmd_queue (IMAP_DATA* idata, const char* cmdstr) { IMAP_COMMAND* cmd; - char* out; - int outlen; - int rc; + unsigned int cmdlen; if (idata->status == IMAP_FATAL) { @@ -80,15 +78,37 @@ int imap_cmd_start (IMAP_DATA* idata, const char* cmdstr) if (!(cmd = cmd_new (idata))) return IMAP_CMD_BAD; - + /* seq, space, cmd, \r\n\0 */ - outlen = strlen (cmd->seq) + strlen (cmdstr) + 4; - out = (char*) safe_malloc (outlen); - snprintf (out, outlen, "%s %s\r\n", cmd->seq, cmdstr); + cmdlen = strlen (cmd->seq) + strlen (cmdstr) + 4; + if (idata->cmdbuflen < cmdlen + (idata->cmdtail - idata->cmdbuf)) + { + unsigned int tailoff = idata->cmdtail - idata->cmdbuf; + safe_realloc (&idata->cmdbuf, tailoff + cmdlen); + idata->cmdbuflen = tailoff + cmdlen; + idata->cmdtail = idata->cmdbuf + tailoff; + } + snprintf (idata->cmdtail, cmdlen, "%s %s\r\n", cmd->seq, cmdstr); + idata->cmdtail += cmdlen - 1; + + return 0; +} + +/* imap_cmd_start: Given an IMAP command, send it to the server. + * If cmdstr is NULL, sends queued commands. */ +int imap_cmd_start (IMAP_DATA* idata, const char* cmdstr) +{ + int rc; + + if (cmdstr && (rc = imap_cmd_queue (idata, cmdstr)) < 0) + return rc; - rc = mutt_socket_write (idata->conn, out); + /* don't write old or empty commands */ + if (idata->cmdtail == idata->cmdbuf) + return IMAP_CMD_BAD; - FREE (&out); + rc = mutt_socket_write (idata->conn, idata->cmdbuf); + idata->cmdtail = idata->cmdbuf; return (rc < 0) ? IMAP_CMD_BAD : 0; } @@ -189,9 +209,6 @@ int imap_code (const char *s) */ int imap_exec (IMAP_DATA* idata, const char* cmdstr, int flags) { - IMAP_COMMAND* cmd; - char* out; - int outlen; int rc; if (idata->status == IMAP_FATAL) @@ -200,17 +217,16 @@ int imap_exec (IMAP_DATA* idata, const char* cmdstr, int flags) return -1; } - if (!(cmd = cmd_new (idata))) - return -1; - - /* seq, space, cmd, \r\n\0 */ - outlen = strlen (cmd->seq) + strlen (cmdstr) + 4; - out = (char*) safe_malloc (outlen); - snprintf (out, outlen, "%s %s\r\n", cmd->seq, cmdstr); - - rc = mutt_socket_write_d (idata->conn, out, + if (cmdstr && (rc = imap_cmd_queue (idata, cmdstr)) < 0) + return rc; + + /* don't write old or empty commands */ + if (idata->cmdtail == idata->cmdbuf) + return IMAP_CMD_BAD; + + rc = mutt_socket_write_d (idata->conn, idata->cmdbuf, flags & IMAP_CMD_PASS ? IMAP_LOG_PASS : IMAP_LOG_CMD); - FREE (&out); + idata->cmdtail = idata->cmdbuf; if (rc < 0) { @@ -407,6 +423,8 @@ static int cmd_handle_untagged (IMAP_DATA* idata) cmd_parse_myrights (idata, s); else if (ascii_strncasecmp ("SEARCH", s, 6) == 0) cmd_parse_search (idata, s); + else if (ascii_strncasecmp ("STATUS", s, 6) == 0) + cmd_parse_status (idata, s); else if (ascii_strncasecmp ("BYE", s, 3) == 0) { dprint (2, (debugfile, "Handling BYE\n")); @@ -688,3 +706,71 @@ static void cmd_parse_search (IMAP_DATA* idata, const char* s) idata->ctx->hdrs[uid2msgno (idata, uid)]->matched = 1; } } + +/* first cut: just do buffy update. Later we may wish to cache all + * mailbox information, even that not desired by buffy */ +static void cmd_parse_status (IMAP_DATA* idata, char* s) +{ + char* mailbox; + BUFFY* inc; + IMAP_MBOX mx; + int count; + + dprint (2, (debugfile, "Handling STATUS\n")); + + mailbox = imap_next_word (s); + s = imap_next_word (mailbox); + *(s - 1) = '\0'; + + imap_unmunge_mbox_name (mailbox); + + for (inc = Incoming; inc; inc = inc->next) + { + if (inc->magic != M_IMAP) + continue; + + if (imap_parse_path (inc->path, &mx) < 0) + { + dprint (1, (debugfile, "Error parsing mailbox %s, skipping\n", inc->path)); + continue; + } + + if (mutt_account_match (&idata->conn->account, &mx.account) && mx.mbox + && strncmp (mailbox, mx.mbox, strlen (mailbox)) == 0) + { + if (*s++ != '(') + { + dprint (1, (debugfile, "Error parsing STATUS\n")); + FREE (&mx.mbox); + return; + } + + while (*s && *s != ')') + { + if (!ascii_strncmp ("RECENT", s, 6)) + { + s = imap_next_word (s); + count = strtol (s, &s, 10); + dprint (2, (debugfile, "%d recent in %s\n", count, mx.mbox)); + inc->new = count; + } + else if (!ascii_strncmp ("UNSEEN", s, 6)) + { + s = imap_next_word (s); + count = strtol (s, &s, 10); + dprint (2, (debugfile, "%d unseen in %s\n", count, mx.mbox)); + } + else if (!ascii_strncmp ("MESSAGES", s, 8)) + { + s = imap_next_word (s); + count = strtol (s, &s, 10); + dprint (2, (debugfile, "%d messages in %s\n", count, mx.mbox)); + } + } + + return; + } + + FREE (&mx.mbox); + } +} diff --git a/imap/imap.c b/imap/imap.c index 89b2af2c..71212dc0 100644 --- a/imap/imap.c +++ b/imap/imap.c @@ -1204,6 +1204,122 @@ int imap_check_mailbox (CONTEXT *ctx, int *index_hint, int force) return result; } +/* split path into (idata,mailbox name) */ +static int imap_buffy_split (const char* path, IMAP_DATA** hidata, char* buf, size_t blen) +{ + IMAP_MBOX mx; + + if (imap_parse_path (path, &mx)) + { + dprint (1, (debugfile, "imap_split_path: Error parsing %s\n", path)); + return -1; + } + if (!(*hidata = imap_conn_find (&(mx.account), option (OPTIMAPPASSIVE) ? M_IMAP_CONN_NONEW : 0))) + { + FREE (&mx.mbox); + return -1; + } + + imap_fix_path (*hidata, mx.mbox, buf, blen); + FREE (&mx.mbox); + + return 0; +} + +/* check for new mail in any subscribed mailboxes. Given a list of mailboxes + * rather than called once for each so that it can batch the commands and + * save on round trips. Returns number of mailboxes with new mail. */ +int imap_buffy_check (int force) +{ + IMAP_DATA* idata; + IMAP_DATA* lastdata = NULL; + BUFFY* mailbox; + char name[LONG_STRING]; + char command[LONG_STRING]; + char munged[LONG_STRING]; + int buffies = 0; + int rc; + + for (mailbox = Incoming; mailbox; mailbox = mailbox->next) + { + /* Init newly-added mailboxes */ + if (! mailbox->magic) + { + if (mx_is_imap (mailbox->path)) + mailbox->magic = M_IMAP; + } + + if (mailbox->magic != M_IMAP) + continue; + + mailbox->new = 0; + + if (imap_buffy_split (mailbox->path, &idata, name, sizeof (name)) < 0) + continue; + + /* Don't issue STATUS on the selected mailbox, it will be NOOPed or + * IDLEd elsewhere */ + if (mutt_strcmp (name, idata->mailbox) == 0 + || (ascii_strcasecmp (name, "INBOX") == 0 + && mutt_strcasecmp (name, idata->mailbox) == 0)) + continue; + + if (!lastdata) + lastdata = idata; + + if (idata != lastdata) + { + /* Send commands to previous server. Sorting the buffy list + * may prevent some infelicitous interleavings */ + if (imap_exec (lastdata, NULL, 0) != IMAP_CMD_OK) + dprint (1, (debugfile, "Error polling mailboxes\n")); + + lastdata = NULL; + } + + if (!mutt_bit_isset (idata->capabilities, IMAP4REV1) && + !mutt_bit_isset (idata->capabilities, STATUS)) + { + dprint (2, (debugfile, "Server doesn't support STATUS\n")); + continue; + } + + imap_munge_mbox_name (munged, sizeof (munged), name); + /* we need a better way to detect new mail... */ + snprintf (command, sizeof (command), "STATUS %s (RECENT)", munged); + + if (imap_cmd_queue (idata, command) < 0) + { + /* pipeline must be full, drain it */ + dprint (2, (debugfile, "IMAP command pipeline full, draining\n")); + + if (imap_exec (idata, NULL, 0) != IMAP_CMD_OK) + dprint (1, (debugfile, "Error polling mailboxes\n")); + + if (imap_cmd_queue (idata, command) < 0) { + /* real trouble */ + dprint (1, (debugfile, "Error queueing command\n")); + return 0; + } + } + } + + if (lastdata && (imap_exec (lastdata, NULL, 0) != IMAP_CMD_OK)) + { + dprint (1, (debugfile, "Error polling mailboxes")); + return 0; + } + + /* collect results */ + for (mailbox = Incoming; mailbox; mailbox = mailbox->next) + { + if (mailbox->magic == M_IMAP && mailbox->new) + buffies++; + } + + return buffies; +} + /* returns count of recent messages if new = 1, else count of total messages. * (useful for at least postponed function) * Question of taste: use RECENT or UNSEEN for new? @@ -1217,11 +1333,10 @@ int imap_mailbox_check (char* path, int new) char buf[LONG_STRING]; char mbox[LONG_STRING]; char mbox_unquoted[LONG_STRING]; - char *s; - int msgcount = 0; int connflags = 0; IMAP_MBOX mx; int rc; + BUFFY* buffy; if (imap_parse_path (path, &mx)) return -1; @@ -1266,39 +1381,17 @@ int imap_mailbox_check (char* path, int new) imap_cmd_start (idata, buf); - do - { - if ((rc = imap_cmd_step (idata)) != IMAP_CMD_CONTINUE) - break; + do + rc = imap_cmd_step (idata); + while (rc == IMAP_CMD_CONTINUE); - s = imap_next_word (idata->buf); - if (ascii_strncasecmp ("STATUS", s, 6) == 0) - { - s = imap_next_word (s); - /* The mailbox name may or may not be quoted here. We could try to - * munge the server response and compare with quoted (or vise versa) - * but it is probably more efficient to just strncmp against both. */ - if (mutt_strncmp (mbox_unquoted, s, mutt_strlen (mbox_unquoted)) == 0 - || mutt_strncmp (mbox, s, mutt_strlen (mbox)) == 0) - { - s = imap_next_word (s); - s = imap_next_word (s); - if (isdigit ((unsigned char) *s)) - { - if (*s != '0') - { - msgcount = atoi(s); - dprint (2, (debugfile, "%d new messages in %s\n", msgcount, path)); - } - } - } - else - dprint (1, (debugfile, "imap_mailbox_check: STATUS response doesn't match requested mailbox.\n")); - } + for (buffy = Incoming; buffy; buffy = buffy->next) + { + if (!strncmp (buffy->path, path, strlen (path))) + return buffy->new; } - while (rc == IMAP_CMD_CONTINUE); - return msgcount; + return 0; } /* returns number of patterns in the search that should be done server-side diff --git a/imap/imap.h b/imap/imap.h index 729bc3f7..b09d07b3 100644 --- a/imap/imap.h +++ b/imap/imap.h @@ -39,7 +39,7 @@ int imap_open_mailbox (CONTEXT *ctx); int imap_open_mailbox_append (CONTEXT *ctx); int imap_sync_mailbox (CONTEXT *ctx, int expunge, int *index_hint); void imap_close_mailbox (CONTEXT *ctx); -int imap_buffy_check (char *path); +int imap_buffy_check (int force); int imap_mailbox_check (char *path, int new); int imap_search (CONTEXT* ctx, const pattern_t* pat); int imap_subscribe (char *path, int subscribe); diff --git a/imap/imap_private.h b/imap/imap_private.h index 8dfc2acf..848ac983 100644 --- a/imap/imap_private.h +++ b/imap/imap_private.h @@ -50,8 +50,9 @@ /* number of entries in the hash table */ #define IMAP_CACHE_LEN 10 -/* number of commands that can be batched into a single request */ -#define IMAP_PIPELINE_DEPTH 10 +/* number of commands that can be batched into a single request + * ( - 1, for the easy way to detect ring buffer wrap) */ +#define IMAP_PIPELINE_DEPTH 11 #define SEQLEN 5 @@ -177,6 +178,9 @@ typedef struct IMAP_COMMAND cmds[IMAP_PIPELINE_DEPTH]; int nextcmd; int lastcmd; + char* cmdbuf; + char* cmdtail; + unsigned int cmdbuflen; /* The following data is all specific to the currently SELECTED mbox */ char delim; @@ -218,6 +222,7 @@ int imap_sync_message (IMAP_DATA *idata, HEADER *hdr, BUFFER *cmd, int imap_authenticate (IMAP_DATA* idata); /* command.c */ +int imap_cmd_queue (IMAP_DATA* idata, const char* cmdstr); int imap_cmd_start (IMAP_DATA* idata, const char* cmd); int imap_cmd_step (IMAP_DATA* idata); void imap_cmd_finish (IMAP_DATA* idata); diff --git a/imap/util.c b/imap/util.c index 8d5da196..ceea64e7 100644 --- a/imap/util.c +++ b/imap/util.c @@ -325,7 +325,7 @@ int imap_get_literal_count(const char *buf, long *bytes) * the qualifier message. Used by imap_copy_message for TRYCREATE */ char* imap_get_qualifier (char* buf) { - const char *s = buf; + char *s = buf; /* skip tag */ s = imap_next_word (s); -- 2.40.0