Use sorted headers in imap_exec_msgset. Fixes [e68f79fef249].

Closes #3000 again.

diff --git a/imap/command.c b/imap/command.c
index 95269bbe7dc078d1cb1b82a09a4e3e34f336ab01..c041336eaf1e5eac07a2fca006742c2dd63218d0 100644 (file)
--- a/imap/command.c
+++ b/imap/command.c
@@ -196,6 +196,7 @@ int imap_code (const char *s)
  *   IMAP_CMD_FAIL_OK: the calling procedure can handle failure. This is used
  *     for checking for a mailbox on append and login
  *   IMAP_CMD_PASS: command contains a password. Suppress logging.
+ *   IMAP_CMD_QUEUE: only queue command, do not execute.
  * Return 0 on success, -1 on Failure, -2 on OK Failure
  */
 int imap_exec (IMAP_DATA* idata, const char* cmdstr, int flags)

diff --git a/imap/imap.c b/imap/imap.c
index 8cf174c61d26a366c5cf36bc4b97e93101edc145..cb7a2eba720b1756a3d4af78501a43f969ade6f4 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -856,40 +856,23 @@ int imap_has_flag (LIST* flag_list, const char* flag)
   return 0;
 }
 
-/* imap_make_msg_set: make an IMAP4rev1 UID message set out of a set of
- *   headers, given a flag enum to filter on.
- * Params: idata: IMAP_DATA containing context containing header set
- *         buf: to write message set into
- *         flag: enum of flag type on which to filter
- *         changed: include only changed messages in message set
- *         invert: invert sense of flag, eg M_READ matches unread messages
- * Returns: number of messages in message set (0 if no matches) */
-int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed,
-                       int invert)
+/* Note: headers must be in SORT_ORDER. See imap_exec_msgset for args.
+ * Pos is an opaque pointer a la strtok. It should be 0 at first call. */
+static int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag,
+                              int changed, int invert, int* pos)
 {
-  HEADER** hdrs;       /* sorted local copy */
+  HEADER** hdrs = idata->ctx->hdrs;
   int count = 0;       /* number of messages in message set */
   int match = 0;       /* whether current message matches flag condition */
   unsigned int setstart = 0;   /* start of current message range */
   int n;
-  short oldsort;       /* we clobber reverse, must restore it */
   int started = 0;
 
-  if (Sort != SORT_ORDER)
-  {
-    hdrs = safe_calloc (idata->ctx->msgcount, sizeof (HEADER*));
-    memcpy (hdrs, idata->ctx->hdrs, idata->ctx->msgcount * sizeof (HEADER*));
-    
-    oldsort = Sort;
-    Sort = SORT_ORDER;
-    qsort ((void*) hdrs, idata->ctx->msgcount, sizeof (HEADER*),
-      mutt_get_sort_func (SORT_ORDER));
-    Sort = oldsort;
-  }
-  else
-    hdrs = idata->ctx->hdrs;
+  hdrs = idata->ctx->hdrs;
   
-  for (n = 0; n < idata->ctx->msgcount; n++)
+  for (n = *pos;
+       n < idata->ctx->msgcount && buf->dptr - buf->data < IMAP_MAX_CMDLEN;
+       n++)
   {
     match = 0;
     /* don't include pending expunged messages */
@@ -951,12 +934,87 @@ int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed,
     }
   }
 
-  if (Sort != SORT_ORDER)
-    FREE (&hdrs);
+  *pos = n;
 
   return count;
 }
 
+/* Prepares commands for all messages matching conditions (must be flushed
+ * with imap_exec)
+ * Params:
+ *   idata: IMAP_DATA containing context containing header set
+ *   pre, post: commands are of the form "%s %s %s %s", tag,
+ *     pre, message set, post
+ *   flag: enum of flag type on which to filter
+ *   changed: include only changed messages in message set
+ *   invert: invert sense of flag, eg M_READ matches unread messages
+ * Returns: number of matched messages, or -1 on failure */
+int imap_exec_msgset (IMAP_DATA* idata, const char* pre, const char* post,
+                      int flag, int changed, int invert)
+{
+  HEADER** hdrs = NULL;
+  short oldsort;
+  BUFFER* cmd;
+  int pos;
+  int rc;
+  int count = 0;
+
+  if (! (cmd = mutt_buffer_init (NULL)))
+  {
+    dprint (1, (debugfile, "imap_exec_msgset: unable to allocate buffer\n"));
+    return -1;
+  }
+
+  /* We make a copy of the headers just in case resorting doesn't give
+   exactly the original order (duplicate messages?), because other parts of
+   the ctx are tied to the header order. This may be overkill. */
+  oldsort = Sort;
+  if (Sort != SORT_ORDER)
+  {
+    hdrs = idata->ctx->hdrs;
+    idata->ctx->hdrs = safe_malloc (idata->ctx->msgcount * sizeof (HEADER*));
+    memcpy (idata->ctx->hdrs, hdrs, idata->ctx->msgcount * sizeof (HEADER*));
+
+    oldsort = Sort;
+    Sort = SORT_ORDER;
+    qsort (idata->ctx->hdrs, idata->ctx->msgcount, sizeof (HEADER*),
+           mutt_get_sort_func (SORT_ORDER));
+  }
+
+  pos = 0;
+
+  do
+  {
+    cmd->dptr = cmd->data;
+    mutt_buffer_printf (cmd, "%s ", pre);
+    rc = imap_make_msg_set (idata, cmd, flag, changed, invert, &pos);
+    if (rc > 0)
+    {
+      mutt_buffer_printf (cmd, " %s", post);
+      if (imap_exec (idata, cmd->data, IMAP_CMD_QUEUE))
+      {
+        rc = -1;
+        goto out;
+      }
+      count += rc;
+    }
+  }
+  while (rc > 0);
+  
+  rc = count;
+
+out:
+  mutt_buffer_free (&cmd);
+  if (oldsort != Sort)
+  {
+    Sort = oldsort;
+    FREE (&idata->ctx->hdrs);
+    idata->ctx->hdrs = hdrs;
+  }
+
+  return rc;
+}
+
 /* returns 0 if mutt's flags match cached server flags */
 static int compare_flags (HEADER* h)
 {
@@ -977,7 +1035,6 @@ static int compare_flags (HEADER* h)
 }
 
 /* Update the IMAP server to reflect the flags a single message.  */
-
 int imap_sync_message (IMAP_DATA *idata, HEADER *hdr, BUFFER *cmd,
                       int *err_continue)
 {
@@ -1055,35 +1112,30 @@ int imap_sync_message (IMAP_DATA *idata, HEADER *hdr, BUFFER *cmd,
   return 0;
 }
 
-static int sync_helper (IMAP_DATA* idata, BUFFER* buf, int right, int flag,
-                        const char* name)
+static int sync_helper (IMAP_DATA* idata, int right, int flag, const char* name)
 {
-  int rc = 0;
+  int count = 0;
+  int rc;
+
+  char buf[LONG_STRING];
 
   if (!mutt_bit_isset (idata->ctx->rights, right))
     return 0;
-  
+
   if (right == M_ACL_WRITE && !imap_has_flag (idata->flags, name))
     return 0;
 
-  buf->dptr = buf->data;
-  mutt_buffer_addstr (buf, "UID STORE ");
-  if (imap_make_msg_set (idata, buf, flag, 1, 0))
-  {
-    rc++;
-    mutt_buffer_printf (buf, " +FLAGS.SILENT (%s)", name);
-    imap_exec (idata, buf->data, IMAP_CMD_QUEUE);
-  }
-  buf->dptr = buf->data;
-  mutt_buffer_addstr (buf, "UID STORE ");
-  if (imap_make_msg_set (idata, buf, flag, 1, 1))
-  {
-    rc++;
-    mutt_buffer_printf (buf, " -FLAGS.SILENT (%s)", name);
-    imap_exec (idata, buf->data, IMAP_CMD_QUEUE);
-  }
-  
-  return rc;
+  snprintf (buf, sizeof(buf), "+FLAGS.SILENT (%s)", name);
+  if ((rc = imap_exec_msgset (idata, "UID STORE", buf, flag, 1, 0)) < 0)
+    return rc;
+  count += rc;
+
+  buf[0] = '-';
+  if ((rc = imap_exec_msgset (idata, "UID STORE", buf, flag, 1, 1)) < 0)
+    return rc;
+  count += rc;
+
+  return count;
 }
 
 /* update the IMAP server to reflect message changes done within mutt.
@@ -1095,11 +1147,9 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
 {
   IMAP_DATA* idata;
   CONTEXT* appendctx = NULL;
-  BUFFER cmd;
   HEADER* h;
   HEADER** hdrs = NULL;
   int oldsort;
-  int deleted;
   int n;
   int rc;
   
@@ -1118,31 +1168,25 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
   if ((rc = imap_check_mailbox (ctx, index_hint, 0)) != 0)
     return rc;
 
-  memset (&cmd, 0, sizeof (cmd));
-
   /* if we are expunging anyway, we can do deleted messages very quickly... */
   if (expunge && mutt_bit_isset (idata->ctx->rights, M_ACL_DELETE))
   {
-    mutt_buffer_addstr (&cmd, "UID STORE ");
-    deleted = imap_make_msg_set (idata, &cmd, M_DELETED, 1, 0);
+    if ((rc = imap_exec_msgset (idata, "UID STORE", "+FLAGS.SILENT (\\Deleted)",
+                                M_DELETED, 1, 0)) < 0)
+    {
+      mutt_error (_("Expunge failed"));
+      mutt_sleep (1);
+      goto out;
+    }
 
-    /* if we have a message set, then let's delete */
-    if (deleted)
+    if (rc > 0)
     {
-      mutt_message (_("Marking %d messages deleted..."), deleted);
-      mutt_buffer_addstr (&cmd, " +FLAGS.SILENT (\\Deleted)");
       /* mark these messages as unchanged so second pass ignores them. Done
        * here so BOGUS UW-IMAP 4.7 SILENT FLAGS updates are ignored. */
       for (n = 0; n < ctx->msgcount; n++)
-       if (ctx->hdrs[n]->deleted && ctx->hdrs[n]->changed)
-         ctx->hdrs[n]->active = 0;
-      if (imap_exec (idata, cmd.data, 0) != 0)
-      {
-       mutt_error (_("Expunge failed"));
-       mutt_sleep (1);
-       rc = -1;
-       goto out;
-      }
+        if (ctx->hdrs[n]->deleted && ctx->hdrs[n]->changed)
+          ctx->hdrs[n]->active = 0;
+      mutt_message (_("Marking %d messages deleted..."), rc);
     }
   }
 
@@ -1176,9 +1220,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
        if (!appendctx)
          appendctx = mx_open_mailbox (ctx->path, M_APPEND | M_QUIET, NULL);
        if (!appendctx)
-       {
          dprint (1, (debugfile, "imap_sync_mailbox: Error opening mailbox in append mode\n"));
-       }
        else
          _mutt_save_message (h, appendctx, 1, 0, 0);
       }
@@ -1192,7 +1234,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
   /* sync +/- flags for the five flags mutt cares about */
   rc = 0;
 
-  /* presort here to avoid doing 10 resorts in imap_make_msg_set */
+  /* presort here to avoid doing 10 resorts in imap_exec_msgset */
   oldsort = Sort;
   if (Sort != SORT_ORDER)
   {
@@ -1206,11 +1248,11 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
            mutt_get_sort_func (SORT_ORDER));
   }
 
-  rc += sync_helper (idata, &cmd, M_ACL_DELETE, M_DELETED, "\\Deleted");
-  rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_FLAG, "\\Flagged");
-  rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_OLD, "Old");
-  rc += sync_helper (idata, &cmd, M_ACL_SEEN, M_READ, "\\Seen");
-  rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_REPLIED, "\\Answered");
+  rc += sync_helper (idata, M_ACL_DELETE, M_DELETED, "\\Deleted");
+  rc += sync_helper (idata, M_ACL_WRITE, M_FLAG, "\\Flagged");
+  rc += sync_helper (idata, M_ACL_WRITE, M_OLD, "Old");
+  rc += sync_helper (idata, M_ACL_SEEN, M_READ, "\\Seen");
+  rc += sync_helper (idata, M_ACL_WRITE, M_REPLIED, "\\Answered");
 
   if (oldsort != Sort)
   {
@@ -1219,24 +1261,22 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
     ctx->hdrs = hdrs;
   }
 
-  if (rc)
+  if (rc && (imap_exec (idata, NULL, 0) != IMAP_CMD_OK))
   {
-    if ((rc = imap_exec (idata, NULL, 0)) != IMAP_CMD_OK)
+    if (ctx->closing)
     {
-      if (ctx->closing)
+      if (mutt_yesorno (_("Error saving flags. Close anyway?"), 0) == M_YES)
       {
-        if (mutt_yesorno (_("Error saving flags. Close anyway?"), 0) == M_YES)
-        {
-          rc = 0;
-          idata->state = IMAP_AUTHENTICATED;
-          goto out;
-        }
+        rc = 0;
+        idata->state = IMAP_AUTHENTICATED;
+        goto out;
       }
-      else
-        mutt_error _("Error saving flags");
-      goto out;
     }
+    else
+      mutt_error _("Error saving flags");
+    goto out;
   }
+
   for (n = 0; n < ctx->msgcount; n++)
     ctx->hdrs[n]->changed = 0;
   ctx->changed = 0;
@@ -1268,8 +1308,6 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
   rc = 0;
 
  out:
-  if (cmd.data)
-    FREE (&cmd.data);
   if (appendctx)
   {
     mx_fastclose_mailbox (appendctx);

diff --git a/imap/imap_private.h b/imap/imap_private.h
index 330136fd276cbc0bb973d15000216a8a2e5bbde1..122eff4c6cb36267519175cae747574c89011541 100644 (file)
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -55,6 +55,9 @@
 #define IMAP_CACHE_LEN 10
 
 #define SEQLEN 5
+/* maximum length of command lines before they must be split (for
+ * lazy servers) */
+#define IMAP_MAX_CMDLEN 1024
 
 #define IMAP_REOPEN_ALLOW     (1<<0)
 #define IMAP_EXPUNGE_EXPECTED (1<<1)
@@ -225,8 +228,8 @@ int imap_rename_mailbox (IMAP_DATA* idata, IMAP_MBOX* mx, const char* newname);
 IMAP_STATUS* imap_mboxcache_get (IMAP_DATA* idata, const char* mbox,
                                  int create);
 void imap_mboxcache_free (IMAP_DATA* idata);
-int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed,
-                       int invert);
+int imap_exec_msgset (IMAP_DATA* idata, const char* pre, const char* post,
+                      int flag, int changed, int invert);
 int imap_open_connection (IMAP_DATA* idata);
 void imap_close_connection (IMAP_DATA* idata);
 IMAP_DATA* imap_conn_find (const ACCOUNT* account, int flags);

diff --git a/imap/message.c b/imap/message.c
index e7df7bb18ecfcd8b288260a22c5dd6d5bcd1fb0e..e4536ae0e03df1f622abb900dcd05bd8b342981a 100644 (file)
--- a/imap/message.c
+++ b/imap/message.c
@@ -688,13 +688,14 @@ int imap_copy_messages (CONTEXT* ctx, HEADER* h, char* dest, int delete)
 {
   IMAP_DATA* idata;
   BUFFER cmd, sync_cmd;
-  char uid[11];
   char mbox[LONG_STRING];
   char mmbox[LONG_STRING];
+  char prompt[LONG_STRING];
   int rc;
   int n;
   IMAP_MBOX mx;
   int err_continue = M_NO;
+  int triedcreate = 0;
 
   idata = (IMAP_DATA*) ctx->data;
 
@@ -719,89 +720,101 @@ int imap_copy_messages (CONTEXT* ctx, HEADER* h, char* dest, int delete)
   }
   
   imap_fix_path (idata, mx.mbox, mbox, sizeof (mbox));
+  imap_munge_mbox_name (mmbox, sizeof (mmbox), mbox);
 
-  memset (&sync_cmd, 0, sizeof (sync_cmd));
-  memset (&cmd, 0, sizeof (cmd));
-  mutt_buffer_addstr (&cmd, "UID COPY ");
-
-  /* Null HEADER* means copy tagged messages */
-  if (!h)
+  /* loop in case of TRYCREATE */
+  do
   {
-    /* if any messages have attachments to delete, fall through to FETCH
-     * and APPEND. TODO: Copy what we can with COPY, fall through for the
-     * remainder. */
-    for (n = 0; n < ctx->msgcount; n++)
+    memset (&sync_cmd, 0, sizeof (sync_cmd));
+    memset (&cmd, 0, sizeof (cmd));
+
+    /* Null HEADER* means copy tagged messages */
+    if (!h)
     {
-      if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->attach_del)
+      /* if any messages have attachments to delete, fall through to FETCH
+       * and APPEND. TODO: Copy what we can with COPY, fall through for the
+       * remainder. */
+      for (n = 0; n < ctx->msgcount; n++)
       {
-       dprint (3, (debugfile, "imap_copy_messages: Message contains attachments to be deleted\n"));
-       return 1;
+        if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->attach_del)
+        {
+          dprint (3, (debugfile, "imap_copy_messages: Message contains attachments to be deleted\n"));
+          return 1;
+        }
+
+        if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->active &&
+            ctx->hdrs[n]->changed)
+        {
+          rc = imap_sync_message (idata, ctx->hdrs[n], &sync_cmd, &err_continue);
+          if (rc < 0)
+          {
+            dprint (1, (debugfile, "imap_copy_messages: could not sync\n"));
+            goto fail;
+          }
+        }
       }
 
-      if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->active &&
-         ctx->hdrs[n]->changed)
+      rc = imap_exec_msgset (idata, "UID COPY", mmbox, M_TAG, 0, 0);
+      if (!rc)
       {
-       rc = imap_sync_message (idata, ctx->hdrs[n], &sync_cmd, &err_continue);
-       if (rc < 0)
-       {
-         dprint (1, (debugfile, "imap_copy_messages: could not sync\n"));
-         goto fail;
-       }
+        dprint (1, (debugfile, "imap_copy_messages: No messages tagged\n"));
+        goto fail;
       }
+      else if (rc < 0)
+      {
+        dprint (1, (debugfile, "could not queue copy\n"));
+        goto fail;
+      }
+      else
+        mutt_message (_("Copying %d messages to %s..."), rc, mbox);
     }
-
-    rc = imap_make_msg_set (idata, &cmd, M_TAG, 0, 0);
-    if (!rc)
+    else
     {
-      dprint (1, (debugfile, "imap_copy_messages: No messages tagged\n"));
-      goto fail;
-    }
-    mutt_message (_("Copying %d messages to %s..."), rc, mbox);
-  }
-  else
-  {
-    mutt_message (_("Copying message %d to %s..."), h->index+1, mbox);
-    snprintf (uid, sizeof (uid), "%u", HEADER_DATA (h)->uid);
-    mutt_buffer_addstr (&cmd, uid);
+      mutt_message (_("Copying message %d to %s..."), h->index+1, mbox);
+      mutt_buffer_printf (&cmd, "UID COPY %u %s", HEADER_DATA (h)->uid, mmbox);
 
-    if (h->active && h->changed)
-    {
-      rc = imap_sync_message (idata, h, &sync_cmd, &err_continue);
-      if (rc < 0)
+      if (h->active && h->changed)
       {
-       dprint (1, (debugfile, "imap_copy_messages: could not sync\n"));
-       goto fail;
+        rc = imap_sync_message (idata, h, &sync_cmd, &err_continue);
+        if (rc < 0)
+        {
+          dprint (1, (debugfile, "imap_copy_messages: could not sync\n"));
+          goto fail;
+        }
+      }    
+      if ((rc = imap_exec (idata, cmd.data, IMAP_CMD_QUEUE)) < 0)
+      {
+        dprint (1, (debugfile, "could not queue copy\n"));
+        goto fail;
       }
     }
-  }
 
-  /* let's get it on */
-  mutt_buffer_addstr (&cmd, " ");
-  imap_munge_mbox_name (mmbox, sizeof (mmbox), mbox);
-  mutt_buffer_addstr (&cmd, mmbox);
-
-  rc = imap_exec (idata, cmd.data, IMAP_CMD_FAIL_OK);
-  if (rc == -2)
-  {
-    /* bail out if command failed for reasons other than nonexistent target */
-    if (ascii_strncasecmp (imap_get_qualifier (idata->buf), "[TRYCREATE]", 11))
-    {
-      imap_error ("imap_copy_messages", idata->buf);
-      goto fail;
-    }
-    dprint (2, (debugfile, "imap_copy_messages: server suggests TRYCREATE\n"));
-    snprintf (mmbox, sizeof (mmbox), _("Create %s?"), mbox);
-    if (option (OPTCONFIRMCREATE) && mutt_yesorno (mmbox, 1) < 1)
+    /* let's get it on */
+    rc = imap_exec (idata, NULL, IMAP_CMD_FAIL_OK);
+    if (rc == -2)
     {
-      mutt_clear_error ();
-      goto fail;
+      if (triedcreate)
+      {
+        dprint (1, (debugfile, "Already tried to create mailbox %s\n", mbox));
+        break;
+      }
+      /* bail out if command failed for reasons other than nonexistent target */
+      if (ascii_strncasecmp (imap_get_qualifier (idata->buf), "[TRYCREATE]", 11))
+        break;
+      dprint (3, (debugfile, "imap_copy_messages: server suggests TRYCREATE\n"));
+      snprintf (prompt, sizeof (prompt), _("Create %s?"), mbox);
+      if (option (OPTCONFIRMCREATE) && mutt_yesorno (prompt, 1) < 1)
+      {
+        mutt_clear_error ();
+        break;
+      }
+      if (imap_create_mailbox (idata, mbox) < 0)
+        break;
+      triedcreate = 1;
     }
-    if (imap_create_mailbox (idata, mbox) < 0)
-      goto fail;
-
-    /* try again */
-    rc = imap_exec (idata, cmd.data, 0);
   }
+  while (rc == -2);
+
   if (rc != 0)
   {
     imap_error ("imap_copy_messages", idata->buf);