[PATCH v2] Simplify maildir_delayed_parsing().

Embed the DO_SORT() and the call to skip_duplicates() so I can
understand it better.  Then remove the duplicate call to
mutt_buffer_printf().

It seems the sort is delayed until after any initial md entries
without a header were first skipped, to make the sort faster.  There
could be a very small number of "new" entries with a header, and if
they are not at the beginning this could speed the sort up a lot.

However putting the sort inside the loop with a DO_SORT() macro and a
call to skip_duplicates() makes the code over-complicated and hard to
understand.  Place the maildir_sort() call before the loop and add
some comments.

Remove the header_parsed bit, because it is only set inside this loop,
and only checked inside this function.  Since the function is not
called twice on the same md list, the header_parsed_bit field is not
needed.

Commit f2eef427 seems to indicate adding the sort was for reducing
seek time.  I'm not a good judge of whether that's effective anymore,
but I added a comment to at least indicate why the sort is being done.
---

This seems to be working, so I'll just send it out now.

The patch should look similar to the #3 and #4 from the last version.

The only difference is that I added back the "skip initial md entries
without md->h" before the sort.  I think in many cases, for maildir,
there may be a small number of new entries.  It's a bit random where
they are, but I'm still going to keep the optimization for now.

 mh.c | 96 +++++++++++++++++++++---------------------------------------
 1 file changed, 33 insertions(+), 63 deletions(-)

diff --git a/mh.c b/mh.c
index a74e94fb..ec9d07c6 100644
--- a/mh.c
+++ b/mh.c
@@ -65,7 +65,6 @@ struct maildir
 {
   HEADER *h;
   char *canon_fname;
-  unsigned header_parsed:1;
 #ifdef HAVE_DIRENT_D_INO
   ino_t inode;
 #endif /* HAVE_DIRENT_D_INO */
@@ -1101,40 +1100,18 @@ static void mh_sort_natural(CONTEXT *ctx, struct maildir **md)
   *md = maildir_sort(*md, (size_t) -1, md_cmp_path);
 }
 
-#if HAVE_DIRENT_D_INO
-static struct maildir *skip_duplicates(struct maildir *p, struct maildir **last)
-{
-  /*
-   * Skip ahead to the next non-duplicate message.
-   *
-   * p should never reach NULL, because we couldn't have reached this point unless
-   * there was a message that needed to be parsed.
-   *
-   * the check for p->header_parsed is likely unnecessary since the dupes will most
-   * likely be at the head of the list.  but it is present for consistency with
-   * the check at the top of the for() loop in maildir_delayed_parsing().
-   */
-  while (!p->h || p->header_parsed)
-  {
-    *last = p;
-    p = p->next;
-  }
-  return p;
-}
-#endif
-
 /*
  * This function does the second parsing pass
  */
-static void maildir_delayed_parsing(CONTEXT * ctx, struct maildir **md,
+static void maildir_delayed_parsing(CONTEXT *ctx, struct maildir **md,
                                     progress_t *progress)
 {
-  struct maildir *p, *last = NULL;
+  struct maildir *p;
+#if HAVE_DIRENT_D_INO
+  struct maildir *last = NULL;
+#endif
   BUFFER *fn = NULL;
   int count;
-#if HAVE_DIRENT_D_INO
-  int sort = 0;
-#endif
 #if USE_HCACHE
   header_cache_t *hc = NULL;
   void *data;
@@ -1143,46 +1120,41 @@ static void maildir_delayed_parsing(CONTEXT * ctx, struct maildir **md,
   int ret;
 #endif
 
-#if HAVE_DIRENT_D_INO
-#define DO_SORT()                                                       \
-  do                                                                    \
-  {                                                                     \
-    if (!sort)                                                          \
-    {                                                                   \
-      muttdbg(4, "maildir: need to sort %s by inode", ctx->path);       \
-      p = maildir_sort(p, (size_t) -1, md_cmp_inode);                   \
-      if (!last)                                                        \
-        *md = p;                                                        \
-      else                                                              \
-        last->next = p;                                                 \
-      sort = 1;                                                         \
-      p = skip_duplicates(p, &last);                                    \
-      mutt_buffer_printf(fn, "%s/%s", ctx->path, p->h->path);           \
-    }                                                                   \
-  } while (0)
-#else
-#define DO_SORT()       /* nothing */
-#endif
-
 #if USE_HCACHE
   hc = mutt_hcache_open(HeaderCache, ctx->path, NULL);
 #endif
-
   fn = mutt_buffer_pool_get();
+  p = *md;
 
-  for (p = *md, count = 0; p; p = p->next, count++)
+  /*
+   * If available, sort by inode number to reduce seek time.
+   */
+#if HAVE_DIRENT_D_INO
+  /* Skip over any initial entries without a header to make sorting faster. */
+  for (; p && !p->h; p = p->next)
+    last = p;
+  if (!p)
+    goto cleanup;
+
+  muttdbg(4, "sorting %s by inode", ctx->path);
+  p = maildir_sort(p, (size_t) -1, md_cmp_inode);
+
+  /* Reattach the initial entries without a header, if any, to the sorted list.
+   * This is needed so that md is properly freed by the caller. */
+  if (last)
+    last->next = p;
+  else
+    *md = p;
+#endif
+
+  for (count = 0; p; p = p->next, count++)
   {
-    if (! (p && p->h && !p->header_parsed))
-    {
-      last = p;
+    if (!p->h)
       continue;
-    }
 
     if (!ctx->quiet && progress)
       mutt_progress_update(progress, count, -1);
 
-    DO_SORT();
-
     mutt_buffer_printf(fn, "%s/%s", ctx->path, p->h->path);
 
 #if USE_HCACHE
@@ -1215,7 +1187,6 @@ static void maildir_delayed_parsing(CONTEXT * ctx, struct maildir **md,
 
       if (maildir_parse_message(ctx->magic, mutt_b2s(fn), p->h->old, p->h))
       {
-        p->header_parsed = 1;
 #if USE_HCACHE
         if (ctx->magic == MUTT_MH)
           mutt_hcache_store(hc, p->h->path, p->h, 0, strlen, MUTT_GENERATE_UIDVALIDITY);
@@ -1229,16 +1200,15 @@ static void maildir_delayed_parsing(CONTEXT * ctx, struct maildir **md,
     }
     mutt_hcache_free(&data);
 #endif
-    last = p;
   }
+
+#if HAVE_DIRENT_D_INO
+cleanup:
+#endif
 #if USE_HCACHE
   mutt_hcache_close(hc);
 #endif
-
   mutt_buffer_pool_release(&fn);
-
-#undef DO_SORT
-
   mh_sort_natural(ctx, md);
 }
 
-- 
2.55.0