Changeset 5501:e0f0a7915711 for imap

Timestamp:

2008-08-29 16:54:56 (3 months ago)

Author:

Brendan Cully <brendan@…>

Branch:

HEAD

Message:

Use sorted headers in imap_exec_msgset. Fixes [e68f79fef249].
Closes #3000 again.

Location:

imap

Files:

• command.c (modified) (1 diff)
• imap.c (modified) (11 diffs)
• imap_private.h (modified) (2 diffs)
• message.c (modified) (2 diffs)

imap/command.c

@@ -197 +197 @@
  *     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
  */

imap/imap.c

@@ -857 +857 @@
 }
 
-/* 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)
-{
-  HEADER** hdrs;        /* sorted local copy */
+/* 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 = 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;
@@ -952 +935 @@
   }
 
+  *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)
-    FREE (&hdrs);
-
-  return count;
+  {
+    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;
 }
 
@@ -978 +1036 @@
 
 /* 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)
@@ -1056 +1113 @@
 }
 
-static int sync_helper (IMAP_DATA* idata, BUFFER* buf, int right, int flag,
-                        const char* name)
-{
-  int rc = 0;
+static int sync_helper (IMAP_DATA* idata, int right, int flag, const char* name)
+{
+  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;
 }
 
@@ -1096 +1148 @@
   IMAP_DATA* idata;
   CONTEXT* appendctx = NULL;
-  BUFFER cmd;
   HEADER* h;
   HEADER** hdrs = NULL;
   int oldsort;
-  int deleted;
   int n;
   int rc;
@@ -1119 +1169 @@
     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 we have a message set, then let's delete */
-    if (deleted)
-    {
-      mutt_message (_("Marking %d messages deleted..."), deleted);
-      mutt_buffer_addstr (&cmd, " +FLAGS.SILENT (\\Deleted)");
+    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 (rc > 0)
+    {
       /* 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);
     }
   }
@@ -1177 +1221 @@
           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);
@@ -1193 +1235 @@
   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)
@@ -1207 +1249 @@
   }
 
-  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)
@@ -1220 +1262 @@
   }
 
-  if (rc)
-  {
-    if ((rc = imap_exec (idata, NULL, 0)) != IMAP_CMD_OK)
-    {
-      if (ctx->closing)
+  if (rc && (imap_exec (idata, NULL, 0) != IMAP_CMD_OK))
+  {
+    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;
@@ -1269 +1309 @@
 
  out:
-  if (cmd.data)
-    FREE (&cmd.data);
   if (appendctx)
   {

imap/imap_private.h

@@ -56 +56 @@
 
 #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)
@@ -226 +229 @@
                                  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/message.c

@@ -689 +689 @@
   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;
@@ -720 +721 @@
   
   imap_fix_path (idata, mx.mbox, mbox, sizeof (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)
-  {
-    /* 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++)
-    {
-      if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->attach_del)
+  imap_munge_mbox_name (mmbox, sizeof (mmbox), mbox);
+
+  /* loop in case of TRYCREATE */
+  do
+  {
+    memset (&sync_cmd, 0, sizeof (sync_cmd));
+    memset (&cmd, 0, sizeof (cmd));
+
+    /* Null HEADER* means copy tagged messages */
+    if (!h)
+    {
+      /* 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;
       }
-    }
-
-    rc = imap_make_msg_set (idata, &cmd, M_TAG, 0, 0);
-    if (!rc)
-    {
-      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);
-
-    if (h->active && h->changed)
-    {
-      rc = imap_sync_message (idata, h, &sync_cmd, &err_continue);
-      if (rc < 0)
+      else if (rc < 0)
       {
-        dprint (1, (debugfile, "imap_copy_messages: could not sync\n"));
-        goto fail;
+        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)
-    {
-      mutt_clear_error ();
-      goto fail;
-    }
-    if (imap_create_mailbox (idata, mbox) < 0)
-      goto fail;
-
-    /* try again */
-    rc = imap_exec (idata, cmd.data, 0);
-  }
+      else
+        mutt_message (_("Copying %d messages to %s..."), rc, mbox);
+    }
+    else
+    {
+      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)
+        {
+          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 */
+    rc = imap_exec (idata, NULL, IMAP_CMD_FAIL_OK);
+    if (rc == -2)
+    {
+      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;
+    }
+  }
+  while (rc == -2);
+
   if (rc != 0)
   {