threading fixes v4
David Bremner
david at tethera.net
Tue Sep 4 03:57:06 PDT 2018
This is mainly coding style issues pointed out by Tomi, along with
moving some of the low level list access into two new functions. The
interdiff is a bit noisy since I dropped the creation of
debug_print.{c,h}.
I'm going to optimistically claim this version is ready to go, but
feel free to point out any issues.
diff --git a/command-line-arguments.c b/command-line-arguments.c
index 90d69453..d64aa85b 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -1,7 +1,7 @@
#include <assert.h>
#include <string.h>
#include <stdio.h>
-#include "debug_print.h"
+#include "error_util.h"
#include "command-line-arguments.h"
typedef enum {
diff --git a/lib/message-id.c b/lib/message-id.c
index a1dce9c8..e71ce9f4 100644
--- a/lib/message-id.c
+++ b/lib/message-id.c
@@ -100,7 +100,6 @@ char *
_notmuch_message_id_parse_strict (void *ctx, const char *message_id)
{
const char *s, *end;
- char *result;
if (message_id == NULL || *message_id == '\0')
return NULL;
@@ -118,11 +117,10 @@ _notmuch_message_id_parse_strict (void *ctx, const char *message_id)
if (*end != '>')
return NULL;
else {
- const char *last = skip_space (end+1);
+ const char *last = skip_space (end + 1);
if (*last != '\0')
return NULL;
}
- result = talloc_strndup (ctx, s, end - s);
- return result;
+ return talloc_strndup (ctx, s, end - s);
}
diff --git a/lib/message.cc b/lib/message.cc
index 5e17029a..6f2f6345 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -637,7 +637,12 @@ _cmpmsg (const void *pa, const void *pb)
time_t time_a = notmuch_message_get_date (*a);
time_t time_b = notmuch_message_get_date (*b);
- return (int) difftime (time_a, time_b);
+ if (time_a == time_b)
+ return 0;
+ else if (time_a < time_b)
+ return -1;
+ else
+ return 1;
}
notmuch_message_list_t *
diff --git a/lib/messages.c b/lib/messages.c
index a88f974f..04fa19f8 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -56,6 +56,15 @@ _notmuch_message_list_add_message (notmuch_message_list_t *list,
list->tail = &node->next;
}
+bool
+_notmuch_message_list_empty (notmuch_message_list_t *list)
+{
+ if (list == NULL)
+ return TRUE;
+
+ return (list->head == NULL);
+}
+
notmuch_messages_t *
_notmuch_messages_create (notmuch_message_list_t *list)
{
@@ -101,6 +110,18 @@ notmuch_messages_valid (notmuch_messages_t *messages)
return (messages->iterator != NULL);
}
+bool
+_notmuch_messages_has_next (notmuch_messages_t *messages)
+{
+ if (! notmuch_messages_valid (messages))
+ return false;
+
+ if (! messages->is_of_list_type)
+ INTERNAL_ERROR("_notmuch_messages_has_next not implimented for msets");
+
+ return (messages->iterator->next != NULL);
+}
+
notmuch_message_t *
notmuch_messages_get (notmuch_messages_t *messages)
{
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 5bbaa292..df32d39c 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -50,12 +50,13 @@ NOTMUCH_BEGIN_DECLS
#include "gmime-extra.h"
#include "xutil.h"
-#include "debug_print.h"
+#include "error_util.h"
#include "string-util.h"
#include "crypto.h"
#ifdef DEBUG
# define DEBUG_DATABASE_SANITY 1
+# define DEBUG_THREADING 1
# define DEBUG_QUERY 1
#endif
@@ -476,6 +477,9 @@ struct _notmuch_messages {
notmuch_message_list_t *
_notmuch_message_list_create (const void *ctx);
+bool
+_notmuch_message_list_empty (notmuch_message_list_t *list);
+
void
_notmuch_message_list_add_message (notmuch_message_list_t *list,
notmuch_message_t *message);
@@ -483,6 +487,9 @@ _notmuch_message_list_add_message (notmuch_message_list_t *list,
notmuch_messages_t *
_notmuch_messages_create (notmuch_message_list_t *list);
+bool
+_notmuch_messages_has_next (notmuch_messages_t *messages);
+
/* query.cc */
bool
diff --git a/lib/thread.cc b/lib/thread.cc
index e4ab4319..47c90664 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -24,6 +24,12 @@
#include <gmime/gmime.h>
#include <glib.h> /* GHashTable */
+#ifdef DEBUG_THREADING
+#define THREAD_DEBUG(format, ...) fprintf(stderr, format " (%s).\n", ##__VA_ARGS__, __location__)
+#else
+#define THREAD_DEBUG(format, ...) do {} while (0) /* ignored */
+#endif
+
#define EMPTY_STRING(s) ((s)[0] == '\0')
struct _notmuch_thread {
@@ -393,10 +399,10 @@ _parent_via_in_reply_to (notmuch_thread_t *thread, notmuch_message_t *message) {
const char *in_reply_to;
in_reply_to = _notmuch_message_get_in_reply_to (message);
- DEBUG_PRINTF("checking message = %s in_reply_to=%s\n",
+ THREAD_DEBUG("checking message = %s in_reply_to=%s\n",
notmuch_message_get_message_id (message), in_reply_to);
- if (in_reply_to && strlen (in_reply_to) &&
+ if (in_reply_to && (! EMPTY_STRING(in_reply_to)) &&
g_hash_table_lookup_extended (thread->message_hash,
in_reply_to, NULL,
(void **) &parent)) {
@@ -416,31 +422,31 @@ _parent_or_toplevel (notmuch_thread_t *thread, notmuch_message_t *message)
const notmuch_string_list_t *references =
_notmuch_message_get_references (message);
- DEBUG_PRINTF("trying to reparent via references: %s\n",
+ THREAD_DEBUG("trying to reparent via references: %s\n",
notmuch_message_get_message_id (message));
for (notmuch_string_node_t *ref_node = references->head;
ref_node; ref_node = ref_node->next) {
- DEBUG_PRINTF("checking reference=%s\n", ref_node->string);
+ THREAD_DEBUG("checking reference=%s\n", ref_node->string);
if ((g_hash_table_lookup_extended (thread->message_hash,
ref_node->string, NULL,
(void **) &new_parent))) {
size_t new_depth = _notmuch_message_get_thread_depth (new_parent);
- DEBUG_PRINTF("got depth %lu\n", new_depth);
+ THREAD_DEBUG("got depth %lu\n", new_depth);
if (new_depth > max_depth || !parent) {
- DEBUG_PRINTF("adding at depth %lu parent=%s\n", new_depth, ref_node->string);
+ THREAD_DEBUG("adding at depth %lu parent=%s\n", new_depth, ref_node->string);
max_depth = new_depth;
parent = new_parent;
}
}
}
if (parent) {
- DEBUG_PRINTF("adding reply %s to parent=%s\n",
+ THREAD_DEBUG("adding reply %s to parent=%s\n",
notmuch_message_get_message_id (message),
notmuch_message_get_message_id (parent));
_notmuch_message_add_reply (parent, message);
} else {
- DEBUG_PRINTF("adding as toplevel %s\n",
+ THREAD_DEBUG("adding as toplevel %s\n",
notmuch_message_get_message_id (message));
_notmuch_message_list_add_message (thread->toplevel_list, message);
}
@@ -475,13 +481,14 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
*/
if (first_node) {
message = first_node->message;
- DEBUG_PRINTF("checking first message %s\n",
+ THREAD_DEBUG("checking first message %s\n",
notmuch_message_get_message_id (message));
- if (! maybe_toplevel_list->head ||
+ if (_notmuch_message_list_empty (maybe_toplevel_list) ||
! _parent_via_in_reply_to (thread, message)) {
- DEBUG_PRINTF("adding first message as toplevel = %s\n",
- notmuch_message_get_message_id (message));
+
+ THREAD_DEBUG("adding first message as toplevel = %s\n",
+ notmuch_message_get_message_id (message));
_notmuch_message_list_add_message (maybe_toplevel_list, message);
}
}
@@ -498,8 +505,7 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
notmuch_messages_valid (roots);
notmuch_messages_move_to_next (roots)) {
notmuch_message_t *message = notmuch_messages_get (roots);
- /* XXX TODO: cleaner way to test iterator for last, list for emptiness */
- if (roots->iterator->next || thread->toplevel_list->head)
+ if (_notmuch_messages_has_next (roots) || ! _notmuch_message_list_empty (thread->toplevel_list))
_parent_or_toplevel (thread, message);
else
_notmuch_message_list_add_message (thread->toplevel_list, message);
diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh
index 4688c0ab..5d6bea7e 100755
--- a/test/T510-thread-replies.sh
+++ b/test/T510-thread-replies.sh
@@ -222,5 +222,4 @@ End of search results.
EOF
test_expect_equal_file EXPECTED OUTPUT
-
test_done
More information about the notmuch
mailing list