[PATCH v2 00/11] Fix search tagging races

* [PATCH v2 00/11] Fix search tagging races
@ 2013-10-24 15:19 Austin Clements
  2013-10-24 15:19 ` [PATCH v2 01/11] schemata: Disambiguate non-terminal names Austin Clements
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Austin Clements @ 2013-10-24 15:19 UTC (permalink / raw)
  To: notmuch

This is v2 of id:1381185201-25197-1-git-send-email-amdragon@mit.edu.
It fixes several comments from Mark and Jani.  This has been rebased
on top of the tag completion changes, so doing * from a large search
buffer will no longer crash.  Hence, this series depends on the
(currently pending) series in
id:1382487721-31776-1-git-send-email-amdragon@mit.edu.

This version does not address what happens when you * on a search
buffer that's still filling.  With this series, it will apply to all
messages that have appeared when the user finishes entering tag
changes.  This isn't ideal, but this seems pretty obscure and I'm not
sure what the right answer is, so I'm punting it to the future.

Another thing that may be worth changing in a follow-up is what
messages * applies to.  Currently, * applies *only* to matched
messages, not all threads in the search, which I think was an accident
of implementation.  This series retains that behavior, but opens up
the possibility of applying to all threads instead.  I think that
would be much more consistent and less surprising behavior.

An approximate diff from v1 is below.  This diff is prior to rebasing,
since the post-rebase diff is not useful.

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index a4eec14..36937fb 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -242,7 +242,11 @@ from TAGS if present."
 	   (error "Changed tag must be of the form `+this_tag' or `-that_tag'")))))
     (sort result-tags 'string<)))
 
-(defconst notmuch-tag-argument-limit 1000)
+(defconst notmuch-tag-argument-limit 1000
+  "Use batch tagging if the tagging query is longer than this.
+
+This limits the length of arguments passed to the notmuch CLI to
+avoid system argument length limits and performance problems.")
 
 (defun notmuch-tag (query &optional tag-changes)
   "Add/remove tags in TAG-CHANGES to messages matching QUERY.
@@ -276,7 +280,6 @@ notmuch-after-tag-hook will be run."
       ;; Use batch tag mode to avoid argument length limitations
       (let ((batch-op (concat (mapconcat #'notmuch-hex-encode tag-changes " ")
 			      " -- " query)))
-	(message "Batch tagging with %s" batch-op)
 	(notmuch-call-notmuch-process :stdin-string batch-op "tag" "--batch")))
     (run-hooks 'notmuch-after-tag-hook))
   ;; in all cases we return tag-changes as a list
diff --git a/notmuch-client.h b/notmuch-client.h
index 1b14910..8bc1a2a 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -144,7 +144,8 @@ chomp_newline (char *str)
 #define NOTMUCH_FORMAT_MIN 1
 /* The minimum non-deprecated structured output format version.
  * Requests for format versions below this will print a stern warning.
- * Must be >= NOTMUCH_FORMAT_MIN and < NOTMUCH_FORMAT_CUR.
+ * Must be between NOTMUCH_FORMAT_MIN and NOTMUCH_FORMAT_CUR,
+ * inclusive.
  */
 #define NOTMUCH_FORMAT_MIN_ACTIVE 1
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 1d14651..7c973b3 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -53,13 +53,13 @@ sanitize_string (const void *ctx, const char *str)
  * NULL. */
 static int
 get_thread_query (notmuch_thread_t *thread,
-		  char **matched_out, char **unmached_out)
+		  char **matched_out, char **unmatched_out)
 {
     notmuch_messages_t *messages;
     char *escaped = NULL;
     size_t escaped_len = 0;
 
-    *matched_out = *unmached_out = NULL;
+    *matched_out = *unmatched_out = NULL;
 
     for (messages = notmuch_thread_get_messages (thread);
 	 notmuch_messages_valid (messages);
@@ -69,17 +69,16 @@ get_thread_query (notmuch_thread_t *thread,
 	const char *mid = notmuch_message_get_message_id (message);
 	/* Determine which query buffer to extend */
 	char **buf = notmuch_message_get_flag (
-	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmached_out;
-	/* Allocate the query buffer is this is the first message */
-	if (!*buf && (*buf = talloc_strdup (thread, "")) == NULL)
-	    return -1;
+	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
 	/* Add this message's id: query.  Since "id" is an exclusive
 	 * prefix, it is implicitly 'or'd together, so we only need to
 	 * join queries with a space. */
 	if (make_boolean_term (thread, "id", mid, &escaped, &escaped_len) < 0)
 	    return -1;
-	*buf = talloc_asprintf_append_buffer (
-	    *buf, "%s%s", **buf ? " " : "", escaped);
+	if (*buf)
+	    *buf = talloc_asprintf_append_buffer (*buf, " %s", escaped);
+	else
+	    *buf = talloc_strdup (thread, escaped);
 	if (!*buf)
 	    return -1;
     }

^ permalink raw reply related	[flat|nested] 15+ messages in thread