[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 665373f..9ed9b51 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;

@@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;

+   if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, _string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, _string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  _string, tag_ops);

if (ret == 0) {
@@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])

 }  while ((line_len = getline (, _size, input)) != -1);

+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree ();

-- 
1.7.10.4



[PATCH 2/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This code is no less correct than the previous version, since it does
not make sense for the array to live longer than the wrapping struct.

By not relying on the context passed into tag_parse_line, we can allow
tag_op_list_t structures to live longer than that context.
---
 notmuch-restore.c |2 +-
 tag-util.c|9 -
 tag-util.h|3 +--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 5a02328..665373f 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -105,7 +105,7 @@ parse_sup_line (void *ctx, char *line,
tok_len++;
}

-   if (tag_op_list_append (ctx, tag_ops, tok, FALSE))
+   if (tag_op_list_append (tag_ops, tok, FALSE))
return -1;
 }

diff --git a/tag-util.c b/tag-util.c
index eab482f..705b7ba 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line,
goto DONE;
}

-   if (tag_op_list_append (ctx, tag_ops, tag, remove)) {
+   if (tag_op_list_append (tag_ops, tag, remove)) {
ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error,
  "aborting");
goto DONE;
@@ -294,7 +294,7 @@ tag_op_list_create (void *ctx)
 list->size = TAG_OP_LIST_INITIAL_SIZE;
 list->count = 0;

-list->ops = talloc_array (ctx, tag_operation_t, list->size);
+list->ops = talloc_array (list, tag_operation_t, list->size);
 if (list->ops == NULL)
return NULL;

@@ -303,8 +303,7 @@ tag_op_list_create (void *ctx)


 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove)
 {
@@ -314,7 +313,7 @@ tag_op_list_append (void *ctx,

 if (list->count == list->size) {
list->size *= 2;
-   list->ops = talloc_realloc (ctx, list->ops, tag_operation_t,
+   list->ops = talloc_realloc (list, list->ops, tag_operation_t,
list->size);
if (list->ops == NULL) {
fprintf (stderr, "Out of memory.\n");
diff --git a/tag-util.h b/tag-util.h
index 99b0fa0..c07bfde 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -87,8 +87,7 @@ tag_op_list_create (void *ctx);
  */

 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove);

-- 
1.7.10.4



[PATCH 1/3] notmuch-restore: fix return value propagation

2012-12-16 Thread da...@tethera.net
From: David Bremner 

Previously notmuch_restore_command returned 0 if tag_message returned
a non-zero (failure) value. This is wrong, since non-zero status
indicates something mysterious went wrong with retrieving the message,
or applying it.

There was also a failure to check or propagate the return value from
tag_op_list_apply in tag_message.
---
 notmuch-restore.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..5a02328 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -25,6 +25,9 @@

 static regex_t regex;

+/* Non-zero return indicates an error in retrieving the message,
+ * or in applying the tags.
+ */
 static int
 tag_message (unused (void *ctx),
 notmuch_database_t *notmuch,
@@ -48,7 +51,7 @@ tag_message (unused (void *ctx),
 /* In order to detect missing messages, this check/optimization is
  * intentionally done *after* first finding the message. */
 if ((flags & TAG_FLAG_REMOVE_ALL) || tag_op_list_size (tag_ops))
-   tag_op_list_apply (message, tag_ops, flags);
+   ret = tag_op_list_apply (message, tag_ops, flags);

 notmuch_message_destroy (message);

@@ -231,8 +234,12 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
if (ret > 0)
continue;

-   if (ret < 0 || tag_message (ctx, notmuch, query_string,
-   tag_ops, flags))
+   if (ret < 0)
+   break;
+
+   ret = tag_message (line_ctx, notmuch, query_string,
+  tag_ops, flags);
+   if (ret)
break;

 }  while ((line_len = getline (, _size, input)) != -1);
-- 
1.7.10.4



v2 restore leak fix

2012-12-16 Thread da...@tethera.net
This obsoletes 

 id:1355688997-19164-1-git-send-email-david at tethera.net

Actually the first patch in the series is now an unrelated bug fix, 
since it isn't needed anymore for 2 and 3.



[RFC PATCH] cli: add --remove-all option to "notmuch tag"

2012-12-16 Thread Jani Nikula
On Sun, 16 Dec 2012, Mark Walters  wrote:
> On Mon, 03 Dec 2012, Jani Nikula  wrote:
>> Add --remove-all option to "notmuch tag" to remove all tags matching
>> query before applying the tag changes. This allows removal and
>> unconditional setting of the tags of a message:
>>
>> $ notmuch tag --remove-all id:foo at example.com
>> $ notmuch tag --remove-all +foo +bar id:foo at example.com
>>
>> without having to resort to the complicated (and still quoting broken):
>>
>> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:foo at 
>> example.com
>> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar 
>> id:foo at example.com
>>
>> ---
>>
>> It's completely untested...
>>
>> This is on top of David's batch tagging branch new-tagging at
>> git://pivot.cs.unb.ca/notmuch.git
>>
>> If there's interest, I'll spin a new version with tests and man page
>> changes after David's stuff has been merged.
>
> I like this: the possibility of setting the tags to something seems
> nice. 

Thanks for the feedback; I'll rebase once we're done with the batch
tagging stuff.

> I am not very keen on the option name but don't have anything much
> better: maybe --remove-all-first or --set-to? 

I still like --remove-all, and, like you say, those aren't much
better. But we can bikeshed this later. ;)

> Incidentally, does this (or should this) give an error if the user calls
> --remove-all with -some_tag (as opposed to +some_tag)?

I think not. There are no errors if you do -foo -foo, or remove a tag
that isn't in any of the messages matching query.

BR,
Jani.


>
> Best wishes
>
> Mark
>
>
>> ---
>>  notmuch-tag.c |   32 
>>  1 file changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/notmuch-tag.c b/notmuch-tag.c
>> index e4fca67..67624cc 100644
>> --- a/notmuch-tag.c
>> +++ b/notmuch-tag.c
>> @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
>> const char *query_string,
>>  notmuch_messages_t *messages;
>>  notmuch_message_t *message;
>>  
>> -/* Optimize the query so it excludes messages that already have
>> - * the specified set of tags. */
>> -query_string = _optimize_tag_query (ctx, query_string, tag_ops);
>> -if (query_string == NULL) {
>> -fprintf (stderr, "Out of memory.\n");
>> -return 1;
>> +if (! (flags & TAG_FLAG_REMOVE_ALL)) {
>> +/* Optimize the query so it excludes messages that already
>> + * have the specified set of tags. */
>> +query_string = _optimize_tag_query (ctx, query_string, tag_ops);
>> +if (query_string == NULL) {
>> +fprintf (stderr, "Out of memory.\n");
>> +return 1;
>> +}
>> +flags |= TAG_FLAG_PRE_OPTIMIZED;
>>  }
>>  
>>  query = notmuch_query_create (notmuch, query_string);
>> @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
>> char *query_string,
>>   notmuch_messages_valid (messages) && ! interrupted;
>>   notmuch_messages_move_to_next (messages)) {
>>  message = notmuch_messages_get (messages);
>> -tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
>> +tag_op_list_apply (message, tag_ops, flags);
>>  notmuch_message_destroy (message);
>>  }
>>  
>> @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  notmuch_config_t *config;
>>  notmuch_database_t *notmuch;
>>  struct sigaction action;
>> -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
>> +tag_op_flag_t flags = TAG_FLAG_NONE;
>>  notmuch_bool_t batch = FALSE;
>> +notmuch_bool_t remove_all = FALSE;
>>  FILE *input = stdin;
>>  char *input_file_name = NULL;
>>  int i, opt_index;
>> @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  notmuch_opt_desc_t options[] = {
>>  { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 },
>>  { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 },
>> +{ NOTMUCH_OPT_BOOLEAN, _all, "remove-all", 0, 0 },
>>  { 0, 0, 0, 0, 0 }
>>  };
>>  
>> @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  }
>>  }
>>  
>> -if (tag_op_list_size (tag_ops) == 0) {
>> +if (tag_op_list_size (tag_ops) == 0 && !remove_all) {
>>  fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to 
>> add or remove.\n");
>>  return 1;
>>  }
>> @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  return 1;
>>  
>>  if (notmuch_config_get_maildir_synchronize_flags (config))
>> -synchronize_flags = TAG_FLAG_MAILDIR_SYNC;
>> +flags |= TAG_FLAG_MAILDIR_SYNC;
>> +
>> +if (remove_all)
>> +flags |= TAG_FLAG_REMOVE_ALL;
>>  
>>  if (batch) {
>>  
>> @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>>  continue;
>>  
>>  if (ret < 0 || tag_query (ctx, notmuch, query_string,
>> -  

[PATCH 3/3] notmuch-restore: use xtalloc version of strndup

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This gives line numbers for better debugging.
---
 notmuch-restore.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..0cc9c9f 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -85,9 +85,10 @@ parse_sup_line (void *ctx, char *line,
return 1;
 }

-*query_str = talloc_strndup (ctx, line + match[1].rm_so,
-match[1].rm_eo - match[1].rm_so);
-file_tags = talloc_strndup (ctx, line + match[2].rm_so,
+*query_str = xtalloc_strndup (ctx, line + match[1].rm_so,
+   match[1].rm_eo - match[1].rm_so);
+
+file_tags = xtalloc_strndup (ctx, line + match[2].rm_so,
match[2].rm_eo - match[2].rm_so);

 char *tok = file_tags;
-- 
1.7.10.4



[PATCH 2/3] util: add xtalloc.[ch]

2012-12-16 Thread da...@tethera.net
From: David Bremner 

These are intended to be simple wrappers to provide slightly better
debugging information than what talloc currently provides natively.
---
 notmuch-client.h|2 +-
 util/Makefile.local |2 +-
 util/xtalloc.c  |   15 +++
 util/xtalloc.h  |   18 ++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 util/xtalloc.c
 create mode 100644 util/xtalloc.h

diff --git a/notmuch-client.h b/notmuch-client.h
index d7b352e..60be030 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include 
 #include 

-#include 
+#include "xtalloc.h"

 #define unused(x) x __attribute__ ((unused))

diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..8a62c00 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)

 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
- $(dir)/string-util.c
+ $(dir)/string-util.c $(dir)/xtalloc.c

 libutil_modules := $(libutil_c_srcs:.c=.o)

diff --git a/util/xtalloc.c b/util/xtalloc.c
new file mode 100644
index 000..22834bd
--- /dev/null
+++ b/util/xtalloc.c
@@ -0,0 +1,15 @@
+#include 
+#include "xtalloc.h"
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_named_const (ctx, len + 1, name);
+
+if (ptr) {
+   memcpy (ptr, str, len);
+   *(ptr + len) = '\0';
+}
+return ptr;
+}
diff --git a/util/xtalloc.h b/util/xtalloc.h
new file mode 100644
index 000..3cc1179
--- /dev/null
+++ b/util/xtalloc.h
@@ -0,0 +1,18 @@
+#ifndef _XTALLOC_H
+#define _XTALLOC_H
+
+#include 
+
+/* Like talloc_strndup, but take an extra parameter for the internal talloc
+ * name (for debugging) */
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name);
+
+/* use the __location__ macro from talloc.h to name a string according to its
+ * source location */
+
+#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, 
len, __location__)
+
+#endif
-- 
1.7.10.4



[PATCH 1/3] CLI: add talloc leak report, controlled by an environment variable.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

The argument handling in notmuch.c seems due for an overhaul, but
until then use an environment variable to specify a location to write
the talloc leak report to.  This is only enabled for the (interesting)
case where some notmuch subcommand is invoked.
---
 notmuch.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 9516dfb..fb49c5a 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -322,8 +322,20 @@ main (int argc, char *argv[])
 for (i = 0; i < ARRAY_SIZE (commands); i++) {
command = [i];

-   if (strcmp (argv[1], command->name) == 0)
-   return (command->function) (local, argc - 1, [1]);
+   if (strcmp (argv[1], command->name) == 0) {
+   int ret;
+   char *talloc_report;
+
+   ret = (command->function) (local, argc - 1, [1]);
+
+   talloc_report=getenv ("NOTMUCH_TALLOC_REPORT");
+   if (talloc_report && strcmp(talloc_report, "") != 0) {
+   FILE *report = fopen (talloc_report, "w");
+   talloc_report_full (NULL, report);
+   }
+
+   return ret;
+   }
 }

 fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
-- 
1.7.10.4



talloc leak debugging

2012-12-16 Thread da...@tethera.net
I was playing a bit with adding talloc leak debugging to notmuch
(since it seems to be essentially free, performance-wise).  I got a bit
discouraged about modifying the argument handling in notmuch.c, so I
hacked it in via an environment variable.



[PATCH v3 5/5] man: document notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 man/man1/notmuch-search.1 |   26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 0aff348..22bcd0a 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -25,9 +25,11 @@ Supported options for
 include
 .RS 4
 .TP 4
-.BR \-\-format= ( json | sexp | text )
+.BR \-\-format= ( json | sexp | text | text0 )

-Presents the results in either JSON, S-Expressions or plain-text (default).
+Presents the results in either JSON, S-Expressions, newline character
+separated plain-text (default), or null character separated plain-text
+(compatible with \fBxargs\fR(1) -0 option where available).
 .RE

 .RS 4
@@ -48,32 +50,36 @@ the authors of the thread and the subject.
 .B threads

 Output the thread IDs of all threads with any message matching the
-search terms, either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or an S-Expression list (\-\-format=sexp).
+search terms, either one per line (\-\-format=text), separated by null
+characters (\-\-format=text0), as a JSON array (\-\-format=json), or
+an S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B messages

 Output the message IDs of all messages matching the search terms,
-either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B files

 Output the filenames of all messages matching the search terms, either
-one per line (\-\-format=text) or as a JSON array (\-\-format=json) or
-as an S-Expression list (\-\-format=sexp).
+one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B tags

 Output all tags that appear on any message matching the search terms,
-either one per line (\-\-format=text) or as a JSON array (\-\-format=json)
-or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RE

-- 
1.7.10.4



[PATCH v3 4/5] test: notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 test/text |   33 +
 1 file changed, 33 insertions(+)

diff --git a/test/text b/test/text
index 428c89b..b5ccefc 100755
--- a/test/text
+++ b/test/text
@@ -52,4 +52,37 @@ output=$(notmuch search --format=text "t?xt-search-m?ssage" 
| notmuch_search_s
 test_expect_equal "$output" \
 "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
text-search-utf8-body-s?bj?ct (inbox unread)"

+add_email_corpus
+
+test_begin_subtest "Search message tags: text0"
+cat < EXPECTED
+attachment inbox signed unread
+EOF
+notmuch search --format=text0 --output=tags '*' | xargs -0 | 
notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+# Use tr(1) to convert --output=text0 to --output=text for
+# comparison. Also translate newlines to spaces to fail with more
+# noise if they are present as delimiters instead of null
+# characters. This assumes there are no newlines in the data.
+test_begin_subtest "Compare text vs. text0 for threads"
+notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > 
EXPECTED
+notmuch search --format=text0 --output=threads '*' | tr "\n\0" " \n" | 
notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "Compare text vs. text0 for messages"
+notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > 
EXPECTED
+notmuch search --format=text0 --output=messages '*' | tr "\n\0" " \n" | 
notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "Compare text vs. text0 for files"
+notmuch search --format=text --output=files '*' | notmuch_search_sanitize > 
EXPECTED
+notmuch search --format=text0 --output=files '*' | tr "\n\0" " \n" | 
notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "Compare text vs. text0 for tags"
+notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > 
EXPECTED
+notmuch search --format=text0 --output=tags '*' | tr "\n\0" " \n" | 
notmuch_search_sanitize > OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
1.7.10.4



[PATCH v3 3/5] cli: add --format=text0 to notmuch search

2012-12-16 Thread Jani Nikula
Add new format text0, which is otherwise the same as text, but use the
null character as separator instead of the newline character. This is
similar to find(1) -print0 option, and works together with the
xargs(1) -0 option.
---
 notmuch-search.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 6218622..627962b 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 int exclude = EXCLUDE_TRUE;
 unsigned int i;

-enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
-   format_sel = NOTMUCH_FORMAT_TEXT;
+enum {
+   NOTMUCH_FORMAT_JSON,
+   NOTMUCH_FORMAT_TEXT,
+   NOTMUCH_FORMAT_TEXT0,
+   NOTMUCH_FORMAT_SEXP
+} format_sel = NOTMUCH_FORMAT_TEXT;

 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_KEYWORD, , "sort", 's',
@@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
  { "sexp", NOTMUCH_FORMAT_SEXP },
  { "text", NOTMUCH_FORMAT_TEXT },
+ { "text0", NOTMUCH_FORMAT_TEXT0 },
  { 0, 0 } } },
{ NOTMUCH_OPT_KEYWORD, , "output", 'o',
  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
@@ -345,6 +350,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 case NOTMUCH_FORMAT_TEXT:
format = sprinter_text_create (ctx, stdout);
break;
+case NOTMUCH_FORMAT_TEXT0:
+   if (output == OUTPUT_SUMMARY) {
+   fprintf (stderr, "Error: --format=text0 is not compatible with 
--output=summary.\n");
+   return 1;
+   }
+   format = sprinter_text0_create (ctx, stdout);
+   break;
 case NOTMUCH_FORMAT_JSON:
format = sprinter_json_create (ctx, stdout);
break;
-- 
1.7.10.4



[PATCH v3 2/5] sprinter: add text0 formatter for null character separated text

2012-12-16 Thread Jani Nikula
Same as the text formatter, but with each field separated by a null
character rather than a newline character.
---
 sprinter-text.c |   22 ++
 sprinter.h  |6 ++
 2 files changed, 28 insertions(+)

diff --git a/sprinter-text.c b/sprinter-text.c
index 10343be..7779488 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -68,6 +68,14 @@ text_separator (struct sprinter *sp)
 }

 static void
+text0_separator (struct sprinter *sp)
+{
+struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+fputc ('\0', sptxt->stream);
+}
+
+static void
 text_set_prefix (struct sprinter *sp, const char *prefix)
 {
 struct sprinter_text *sptxt = (struct sprinter_text *) sp;
@@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream)
 res->stream = stream;
 return >vtable;
 }
+
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream)
+{
+struct sprinter *sp;
+
+sp = sprinter_text_create (ctx, stream);
+if (! sp)
+   return NULL;
+
+sp->separator = text0_separator;
+
+return sp;
+}
diff --git a/sprinter.h b/sprinter.h
index f43a844..f859672 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -67,6 +67,12 @@ typedef struct sprinter {
 struct sprinter *
 sprinter_text_create (const void *ctx, FILE *stream);

+/* Create a new unstructured printer that emits the text format for
+ * "notmuch search", with each field separated by a null character
+ * instead of the newline character. */
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream);
+
 /* Create a new structure printer that emits JSON. */
 struct sprinter *
 sprinter_json_create (const void *ctx, FILE *stream);
-- 
1.7.10.4



[PATCH v3 1/5] sprinter: clarify separator documentation

2012-12-16 Thread Jani Nikula
For text printers, the separator is a syntactic element.
---
 sprinter.h |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sprinter.h b/sprinter.h
index 59776a9..f43a844 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -42,10 +42,11 @@ typedef struct sprinter {
  */
 void (*map_key) (struct sprinter *, const char *);

-/* Insert a separator (usually extra whitespace) for improved
- * readability without affecting the abstract syntax of the
- * structure being printed.
- * For JSON, this could simply be a line break.
+/* Insert a separator (usually extra whitespace). For the text
+ * printers, this is a syntactic separator. For the structured
+ * printers, this is for improved readability without affecting
+ * the abstract syntax of the structure being printed. For JSON,
+ * this could simply be a line break.
  */
 void (*separator) (struct sprinter *);

-- 
1.7.10.4



gmail importer script

2012-12-16 Thread Patrick Totzke
Quoting Jason A. Donenfeld (2012-12-16 20:44:04)
> On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke 
> wrote:
> 
> Well, thats not the point.. the script shouldn't die like this.
> I think it's be better if the script caught that exception, deleted the
> file
> and continued..
> 
> 
> Probably, but I suspect it's related to whatever mystery error you ran into
> before with the corruption.
> 
> Does deleting that file and trying again fix it??

I don't know. How would I know which file to remove?
I just removed the newest files in new/cur/tmp
but this doesnt solve the issue.
What would really help here is some more informative error message!

I tried with the `-d` option but did not see any local path in the output.

> 
> Anyway, this is extremely stable for me and a few others at this point. I'm
> going to wait for other users to report errors. Alternatively, send me patches
> if you want things to happen.

Personally, I don't really care if 'things are happening', as I'm kind of OK 
with my current
offlineimap configuration and would seriously consider switching from OI
only if it meant an improvement in speed or stability.

I test this code because I believe that the notmuch ecosystem
would benefit from a working solution and that reviews and bug-reports
are important.

Of course, I don't expect any miracles in terms of speed at this early stage.
But the bug I reported above should be addressed at some point in order not to
scare off potential users.

Have you considered including your code in offlineimap?
I'm asking because OI already solved some of the problems you might face later 
on
if you want to continue working on this.

* multi-threaded downloads
* the ability to read passwords not as plaintext parameter..

best,
/p
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: signature
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20121216/62774cac/attachment.pgp>


gmail importer script

2012-12-16 Thread Jason A. Donenfeld
On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke wrote:
>
> Well, thats not the point.. the script shouldn't die like this.
> I think it's be better if the script caught that exception, deleted the
> file
> and continued..


Probably, but I suspect it's related to whatever mystery error you ran into
before with the corruption.

Does deleting that file and trying again fix it?


Anyway, this is extremely stable for me and a few others at this point. I'm
going to wait for other users to report errors. Alternatively, send me
patches if you want things to happen.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20121216/e36a1be9/attachment-0001.html>


[PATCH v3 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Mark Walters
On Sun, 16 Dec 2012, Jani Nikula  wrote:
> Hi all, v3 of id:cover.1355064714.git.jani at nikula.org
>
> Changes since v2:
>  * add new patch 1/5 to clarify sprinter documentation
>  * fix the test patch 4/5 according to id:8738z6wguj.fsf at qmul.ac.uk and
>id:87y5gyvvv7.fsf at awakening.csail.mit.edu
>
> Diff to v2 at the end of the cover letter.

LGTM +1

Best wishes

Mark



>
>
> BR,
> Jani.
>
>
> Jani Nikula (5):
>   sprinter: clarify separator documentation
>   sprinter: add text0 formatter for null character separated text
>   cli: add --format=text0 to notmuch search
>   test: notmuch search --format=text0
>   man: document notmuch search --format=text0
>
>  man/man1/notmuch-search.1 |   26 --
>  notmuch-search.c  |   16 ++--
>  sprinter-text.c   |   22 ++
>  sprinter.h|   15 +++
>  test/text |   33 +
>  5 files changed, 96 insertions(+), 16 deletions(-)
>
> -- 
> 1.7.10.4
>
>
>
> diff --git a/sprinter.h b/sprinter.h
> index f36b999..f859672 100644
> --- a/sprinter.h
> +++ b/sprinter.h
> @@ -42,10 +42,11 @@ typedef struct sprinter {
>   */
>  void (*map_key) (struct sprinter *, const char *);
>  
> -/* Insert a separator (usually extra whitespace) for improved
> - * readability without affecting the abstract syntax of the
> - * structure being printed.
> - * For JSON, this could simply be a line break.
> +/* Insert a separator (usually extra whitespace). For the text
> + * printers, this is a syntactic separator. For the structured
> + * printers, this is for improved readability without affecting
> + * the abstract syntax of the structure being printed. For JSON,
> + * this could simply be a line break.
>   */
>  void (*separator) (struct sprinter *);
>  
> diff --git a/test/text b/test/text
> index e003a66..b5ccefc 100755
> --- a/test/text
> +++ b/test/text
> @@ -55,30 +55,34 @@ test_expect_equal "$output" \
>  add_email_corpus
>  
>  test_begin_subtest "Search message tags: text0"
> -cat < EXPECTED.$test_count
> +cat < EXPECTED
>  attachment inbox signed unread
>  EOF
> -notmuch search --format=text0 --output=tags '*' | xargs -0 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +notmuch search --format=text0 --output=tags '*' | xargs -0 | 
> notmuch_search_sanitize > OUTPUT
> +test_expect_equal_file EXPECTED OUTPUT
>  
> +# Use tr(1) to convert --output=text0 to --output=text for
> +# comparison. Also translate newlines to spaces to fail with more
> +# noise if they are present as delimiters instead of null
> +# characters. This assumes there are no newlines in the data.
>  test_begin_subtest "Compare text vs. text0 for threads"
> -notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> -notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
> > EXPECTED
> +notmuch search --format=text0 --output=threads '*' | tr "\n\0" " \n" | 
> notmuch_search_sanitize > OUTPUT
> +test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "Compare text vs. text0 for messages"
> -notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> -notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
> > EXPECTED
> +notmuch search --format=text0 --output=messages '*' | tr "\n\0" " \n" | 
> notmuch_search_sanitize > OUTPUT
> +test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "Compare text vs. text0 for files"
> -notmuch search --format=text --output=files '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> -notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > 
> EXPECTED
> +notmuch search --format=text0 --output=files '*' | tr "\n\0" " \n" | 
> notmuch_search_sanitize > OUTPUT
> +test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "Compare text vs. text0 for tags"
> -notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> -notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +notmuch search --format=text --output=tags '*' | 

[RFC PATCH] cli: add --remove-all option to "notmuch tag"

2012-12-16 Thread Mark Walters

On Mon, 03 Dec 2012, Jani Nikula  wrote:
> Add --remove-all option to "notmuch tag" to remove all tags matching
> query before applying the tag changes. This allows removal and
> unconditional setting of the tags of a message:
>
> $ notmuch tag --remove-all id:foo at example.com
> $ notmuch tag --remove-all +foo +bar id:foo at example.com
>
> without having to resort to the complicated (and still quoting broken):
>
> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:foo at 
> example.com
> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar 
> id:foo at example.com
>
> ---
>
> It's completely untested...
>
> This is on top of David's batch tagging branch new-tagging at
> git://pivot.cs.unb.ca/notmuch.git
>
> If there's interest, I'll spin a new version with tests and man page
> changes after David's stuff has been merged.

I like this: the possibility of setting the tags to something seems
nice. 

I am not very keen on the option name but don't have anything much
better: maybe --remove-all-first or --set-to? 

Incidentally, does this (or should this) give an error if the user calls
--remove-all with -some_tag (as opposed to +some_tag)?

Best wishes

Mark


> ---
>  notmuch-tag.c |   32 
>  1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index e4fca67..67624cc 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
> const char *query_string,
>  notmuch_messages_t *messages;
>  notmuch_message_t *message;
>  
> -/* Optimize the query so it excludes messages that already have
> - * the specified set of tags. */
> -query_string = _optimize_tag_query (ctx, query_string, tag_ops);
> -if (query_string == NULL) {
> - fprintf (stderr, "Out of memory.\n");
> - return 1;
> +if (! (flags & TAG_FLAG_REMOVE_ALL)) {
> + /* Optimize the query so it excludes messages that already
> +  * have the specified set of tags. */
> + query_string = _optimize_tag_query (ctx, query_string, tag_ops);
> + if (query_string == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> + flags |= TAG_FLAG_PRE_OPTIMIZED;
>  }
>  
>  query = notmuch_query_create (notmuch, query_string);
> @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
> char *query_string,
>notmuch_messages_valid (messages) && ! interrupted;
>notmuch_messages_move_to_next (messages)) {
>   message = notmuch_messages_get (messages);
> - tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
> + tag_op_list_apply (message, tag_ops, flags);
>   notmuch_message_destroy (message);
>  }
>  
> @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  notmuch_config_t *config;
>  notmuch_database_t *notmuch;
>  struct sigaction action;
> -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
> +tag_op_flag_t flags = TAG_FLAG_NONE;
>  notmuch_bool_t batch = FALSE;
> +notmuch_bool_t remove_all = FALSE;
>  FILE *input = stdin;
>  char *input_file_name = NULL;
>  int i, opt_index;
> @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  notmuch_opt_desc_t options[] = {
>   { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 },
>   { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 },
> + { NOTMUCH_OPT_BOOLEAN, _all, "remove-all", 0, 0 },
>   { 0, 0, 0, 0, 0 }
>  };
>  
> @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>   }
>   }
>  
> - if (tag_op_list_size (tag_ops) == 0) {
> + if (tag_op_list_size (tag_ops) == 0 && !remove_all) {
>   fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to 
> add or remove.\n");
>   return 1;
>   }
> @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>   return 1;
>  
>  if (notmuch_config_get_maildir_synchronize_flags (config))
> - synchronize_flags = TAG_FLAG_MAILDIR_SYNC;
> + flags |= TAG_FLAG_MAILDIR_SYNC;
> +
> +if (remove_all)
> + flags |= TAG_FLAG_REMOVE_ALL;
>  
>  if (batch) {
>  
> @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>   continue;
>  
>   if (ret < 0 || tag_query (ctx, notmuch, query_string,
> -   tag_ops, synchronize_flags))
> +   tag_ops, flags))
>   break;
>   }
>  
>   if (line)
>   free (line);
>  } else
> - ret = tag_query (ctx, notmuch, query_string, tag_ops, 
> synchronize_flags);
> + ret = tag_query (ctx, notmuch, query_string, tag_ops, flags);
>  
>  notmuch_database_destroy (notmuch);
>  
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing 

[PATCH 4/4] perf-test: add memory leak test for dump restore

2012-12-16 Thread David Bremner
david at tethera.net writes:
> +
> +memory_run 'load nmbug tags' 'notmuch restore --accumulate 
> --input=corpus.tags/nmbug.sup-dump'
> +memory_run 'dump *' 'notmuch dump --output=tags.sup'
> +memory_run 'restore *' 'notmuch restore --input=tags.sup'
> +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag 
> --output=tags.bt'
> +memory_run 'restore --format=batch-tag *' 'notmuch restore 
> --format=batch-tag --input=tags.bt'
> +

We were talking on IRC about how/if valgrind would cope with talloc, and
the possibility that chunks of memory are still reachable by talloc, but
not by user code.  Currently the talloc context "local" in main() is
(slightly perversely) only freed in the case of "return 1", so all the
memory allocated by talloc on that contex is shown as leaked:

 3,005,500 (93 direct, 3,005,407 indirect) bytes in 1 blocks are definitely 
lost in loss record 553 of 553
at 0x4C2A26B: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x55F14C7: talloc_strndup (in 
/usr/lib/x86_64-linux-gnu/libtalloc.so.2.0.7)
by 0x4115E8: parse_sup_line (notmuch-restore.c:90)
by 0x411AD4: notmuch_restore_command (notmuch-restore.c:209)
by 0x40B2A4: main (notmuch.c:294)

Although this is probably a bug in main(), it does point valgrind to the
right culprit.

As our memory allocation is (alas) a mix of talloc, malloc, and
g_malloc, we probably need both valgrind tests, and some way to toggle
talloc memory debugging.  (
http://talloc.samba.org/talloc/doc/html/group__talloc__debug.html )

d




[PATCH v2 1/7] emacs: Centralize notmuch command error handling

2012-12-16 Thread David Bremner
Austin Clements  writes:

> This provides library functions for unified handling of errors from
> the notmuch CLI.  Follow-up patches will convert some scattered error
> handling to use this and add error handling where we currently ignore
> errors.

pushed, 

d


[PATCH 1/7] cli: Framework for structured output versioning

2012-12-16 Thread David Bremner
Austin Clements  writes:

>
> This series of patches introduces a way for CLI callers to request a
> specific format version on the command line and to determine if the
> CLI does not supported the requested version 

pushed, 

d


[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 4b76d83..52e7ecb 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;

@@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;

+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, _string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, _string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  _string, tag_ops);

if (ret == 0) {
@@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
break;
}

+   talloc_free (line_ctx);
+   /* setting to NULL is important to avoid a double free */
+   line_ctx = NULL;
 }  while ((line_len = getline (, _size, input)) != -1);

+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree ();

-- 
1.7.10.4



[PATCH 2/3] notmuch-restore: replace continue with if

2012-12-16 Thread da...@tethera.net
From: David Bremner 

It is maybe a bit counter-intuitive with the condition reversed like
this, but it makes the memory handling fix in the next patch easier.
---
 notmuch-restore.c |   15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index ae0ef45..4b76d83 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -228,12 +228,15 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
}
}

-   if (ret > 0)
-   continue;
-
-   if (ret < 0 || tag_message (ctx, notmuch, query_string,
-   tag_ops, flags))
-   break;
+   if (ret <= 0) {
+   if (ret < 0)
+   break;
+
+   ret = tag_message (line_ctx, notmuch, query_string,
+  tag_ops, flags);
+   if (ret < 0)
+   break;
+   }

 }  while ((line_len = getline (, _size, input)) != -1);

-- 
1.7.10.4



[PATCH 1/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This code is no less correct than the previous version, since it does
not make sense for the array to live longer than the wrapping struct.

By not relying on the context passed into tag_parse_line, we can allow
tag_op_list_t structures to live longer than that context.
---
 notmuch-restore.c |2 +-
 tag-util.c|9 -
 tag-util.h|3 +--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..ae0ef45 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -102,7 +102,7 @@ parse_sup_line (void *ctx, char *line,
tok_len++;
}

-   if (tag_op_list_append (ctx, tag_ops, tok, FALSE))
+   if (tag_op_list_append (tag_ops, tok, FALSE))
return -1;
 }

diff --git a/tag-util.c b/tag-util.c
index eab482f..705b7ba 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line,
goto DONE;
}

-   if (tag_op_list_append (ctx, tag_ops, tag, remove)) {
+   if (tag_op_list_append (tag_ops, tag, remove)) {
ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error,
  "aborting");
goto DONE;
@@ -294,7 +294,7 @@ tag_op_list_create (void *ctx)
 list->size = TAG_OP_LIST_INITIAL_SIZE;
 list->count = 0;

-list->ops = talloc_array (ctx, tag_operation_t, list->size);
+list->ops = talloc_array (list, tag_operation_t, list->size);
 if (list->ops == NULL)
return NULL;

@@ -303,8 +303,7 @@ tag_op_list_create (void *ctx)


 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove)
 {
@@ -314,7 +313,7 @@ tag_op_list_append (void *ctx,

 if (list->count == list->size) {
list->size *= 2;
-   list->ops = talloc_realloc (ctx, list->ops, tag_operation_t,
+   list->ops = talloc_realloc (list, list->ops, tag_operation_t,
list->size);
if (list->ops == NULL) {
fprintf (stderr, "Out of memory.\n");
diff --git a/tag-util.h b/tag-util.h
index 99b0fa0..c07bfde 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -87,8 +87,7 @@ tag_op_list_create (void *ctx);
  */

 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove);

-- 
1.7.10.4



proposed fix for memory leak in notmuch restore

2012-12-16 Thread da...@tethera.net
In id:87vcc2q5n2.fsf at nikula.org, Jani points out a memory leak in the
new restore code.  This series is a proposed fix, namely do any
allocation on a temporary talloc context that is destroyed after each
line is processed.




[PATCH v3 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Austin Clements
On Sun, 16 Dec 2012, Jani Nikula  wrote:
> Hi all, v3 of id:cover.1355064714.git.jani at nikula.org
>
> Changes since v2:
>  * add new patch 1/5 to clarify sprinter documentation
>  * fix the test patch 4/5 according to id:8738z6wguj.fsf at qmul.ac.uk and
>id:87y5gyvvv7.fsf at awakening.csail.mit.edu
>
> Diff to v2 at the end of the cover letter.
>
>
> BR,
> Jani.

LGTM.


[PATCH 4/4] perf-test: add memory leak test for dump restore

2012-12-16 Thread da...@tethera.net
From: David Bremner 

In id:87vcc2q5n2.fsf at nikula.org, Jani points out a memory leak in the
current version of the sup restore code. Among other things, this test
is intended to verify a fix for that leak.
---
 performance-test/M01-dump-restore |   15 +++
 1 file changed, 15 insertions(+)
 create mode 100755 performance-test/M01-dump-restore

diff --git a/performance-test/M01-dump-restore 
b/performance-test/M01-dump-restore
new file mode 100755
index 000..be5894a
--- /dev/null
+++ b/performance-test/M01-dump-restore
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+test_description='dump and restore'
+
+. ./perf-test-lib.sh
+
+memory_start
+
+memory_run 'load nmbug tags' 'notmuch restore --accumulate 
--input=corpus.tags/nmbug.sup-dump'
+memory_run 'dump *' 'notmuch dump --output=tags.sup'
+memory_run 'restore *' 'notmuch restore --input=tags.sup'
+memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag 
--output=tags.bt'
+memory_run 'restore --format=batch-tag *' 'notmuch restore --format=batch-tag 
--input=tags.bt'
+
+memory_done
-- 
1.7.10.4



[PATCH 3/4] perf-test: initial version of memory test infrastructure.

2012-12-16 Thread da...@tethera.net
From: David Bremner 

The idea is run some code under valgrind --leak-check=full and report
a summary, leaving the user to peruse the log file if they want.

We go to some lengths to preserve the log files from accidental
overwriting; the full corpus takes about 3 hours to run under valgrind
on my machine.

The naming of the log directories is probably overkill; I find it nice
to have them sequenced by time. Arguably the mktemp is then overkill,
but I know people will be nervous if it looks like timestamps are
being used for uniqueness.

One new test is included, to check notmuch new for memory leaks.
---
 performance-test/.gitignore   |1 +
 performance-test/M00-new  |   14 +
 performance-test/Makefile.local   |   17 +--
 performance-test/README   |   57 +
 performance-test/perf-test-lib.sh |   55 ---
 5 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100755 performance-test/M00-new

diff --git a/performance-test/.gitignore b/performance-test/.gitignore
index 6421a9a..f3f9be4 100644
--- a/performance-test/.gitignore
+++ b/performance-test/.gitignore
@@ -1,3 +1,4 @@
 tmp.*/
+log.*/
 corpus/
 notmuch.cache.*/
diff --git a/performance-test/M00-new b/performance-test/M00-new
new file mode 100755
index 000..733e9b0
--- /dev/null
+++ b/performance-test/M00-new
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+test_description='notmuch new'
+
+. ./perf-test-lib.sh
+
+# ensure initial 'notmuch new' is run by memory_start
+uncache_database
+
+memory_start
+
+memory_run "notmuch new" "notmuch new"
+
+memory_done
diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
index 57beb44..357d800 100644
--- a/performance-test/Makefile.local
+++ b/performance-test/Makefile.local
@@ -4,14 +4,25 @@ dir := performance-test

 include $(dir)/version.sh

+# these two are just make sure dir is expanded at the right time.
+TIME_TEST_SCRIPT := ${dir}/notmuch-time-test
+MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test
+
 CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz
 TXZFILE := ${dir}/download/${CORPUS_NAME}
 SIGFILE := ${TXZFILE}.asc
-TEST_SCRIPT := ${dir}/notmuch-perf-test
 DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}

+perf-test: time-test memory-test
+
 time-test: setup-perf-test all
-   $(TEST_SCRIPT) $(OPTIONS)
+   @echo
+   $(TIME_TEST_SCRIPT) $(TEST_OPTIONS)
+
+memory-test: setup-perf-test all
+   @echo
+   $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS)
+

 .PHONY: download-corpus setup-perf-test

@@ -29,4 +40,4 @@ $(TXZFILE):
 download-corpus:
wget -O ${TXZFILE} ${DEFAULT_URL}

-CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.*
+CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus 
$(dir)/notmuch.cache.*
diff --git a/performance-test/README b/performance-test/README
index d1fb6de..7eaf5f7 100644
--- a/performance-test/README
+++ b/performance-test/README
@@ -1,3 +1,10 @@
+Performance Tests
+-
+
+This directory contains two kinds of performance tests, time tests,
+and memory tests. The former use gnu time, and the latter use
+valgrind.
+
 Pre-requisites
 --

@@ -5,9 +12,10 @@ In addition to having notmuch, you need:

 - gpg
 - gnu tar
-- gnu time
+- gnu time (for the time tests).
 - xz. Some speedup can be gotten by installing "pixz", but this is
   probably only worthwhile if you are debugging the tests.
+- valgrind (for the memory tests)

 Getting set up to run tests:
 
@@ -36,34 +44,47 @@ for a list of mirrors.
 Running tests
 -

-The easiest way to run performance tests is to say "make time-test", (or
-simply run the notmuch-time-test script). Either command will run all
-available performance tests.
-
-Alternately, you can run a specific subset of tests by simply invoking
-one of the executable scripts in this directory, (such as ./basic).
-Each test script supports the following arguments
+The easiest way to run performance tests is to say "make perf-test".
+This will run all time and memory tests.  Be aware that the memory
+tests are quite time consuming when run on the full corpus, and that
+depending on your interests it may be more sensible to run "make
+time-test" or "make memory-test".  You can also invoke one of the
+scripts notmuch-time-test or notmuch-memory-test or run a more
+specific subset of tests by simply invoking one of the executable
+scripts in this directory, (such as ./T00-new).  Each test script
+supports the following arguments

 --small / --medium / --large   Choose corpus size.
 --debugEnable debugging. In particular don't 
delete
temporary directories.

+When using the make targets, you can pass arguments to all test
+scripts by defining the make variable TEST_OPTIONS.
+
 Writing tests
 -

-Have a look at "T01-dump-restore" 

[PATCH 2/4] perf-test: rename current tests as "time tests"

2012-12-16 Thread da...@tethera.net
From: David Bremner 

This is almost entirely renaming files, except for updating a few
references to those file names, and changing the makefile target.

A new set of memory tests will be run separately because they take
much longer.
---
 performance-test/00-new|   15 ---
 performance-test/01-dump-restore   |   13 -
 performance-test/02-tag|   14 --
 performance-test/Makefile.local|2 +-
 performance-test/README|9 +
 performance-test/T00-new   |   15 +++
 performance-test/T01-dump-restore  |   13 +
 performance-test/T02-tag   |   14 ++
 performance-test/notmuch-perf-test |   27 ---
 performance-test/notmuch-time-test |   27 +++
 10 files changed, 75 insertions(+), 74 deletions(-)
 delete mode 100755 performance-test/00-new
 delete mode 100755 performance-test/01-dump-restore
 delete mode 100755 performance-test/02-tag
 create mode 100755 performance-test/T00-new
 create mode 100755 performance-test/T01-dump-restore
 create mode 100755 performance-test/T02-tag
 delete mode 100755 performance-test/notmuch-perf-test
 create mode 100755 performance-test/notmuch-time-test

diff --git a/performance-test/00-new b/performance-test/00-new
deleted file mode 100755
index 553bb8b..000
--- a/performance-test/00-new
+++ /dev/null
@@ -1,15 +0,0 @@
-#!/bin/bash
-
-test_description='notmuch new'
-
-. ./perf-test-lib.sh
-
-uncache_database
-
-time_start
-
-for i in $(seq 2 6); do
-time_run "notmuch new #$i" 'notmuch new'
-done
-
-time_done
diff --git a/performance-test/01-dump-restore b/performance-test/01-dump-restore
deleted file mode 100755
index b2ff940..000
--- a/performance-test/01-dump-restore
+++ /dev/null
@@ -1,13 +0,0 @@
-#!/bin/bash
-
-test_description='dump and restore'
-
-. ./perf-test-lib.sh
-
-time_start
-
-time_run 'load nmbug tags' 'notmuch restore --accumulate < 
corpus.tags/nmbug.sup-dump'
-time_run 'dump *' 'notmuch dump > tags.out'
-time_run 'restore *' 'notmuch restore < tags.out'
-
-time_done
diff --git a/performance-test/02-tag b/performance-test/02-tag
deleted file mode 100755
index 78ceccc..000
--- a/performance-test/02-tag
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/bin/bash
-
-test_description='tagging'
-
-. ./perf-test-lib.sh
-
-time_start
-
-time_run 'tag * +new_tag' "notmuch tag +new_tag '*'"
-time_run 'tag * +existing_tag' "notmuch tag +new_tag '*'"
-time_run 'tag * -existing_tag' "notmuch tag -new_tag '*'"
-time_run 'tag * -missing_tag' "notmuch tag -new_tag '*'"
-
-time_done
diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
index 3834e4d..57beb44 100644
--- a/performance-test/Makefile.local
+++ b/performance-test/Makefile.local
@@ -10,7 +10,7 @@ SIGFILE := ${TXZFILE}.asc
 TEST_SCRIPT := ${dir}/notmuch-perf-test
 DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}

-perf-test: setup-perf-test all
+time-test: setup-perf-test all
$(TEST_SCRIPT) $(OPTIONS)

 .PHONY: download-corpus setup-perf-test
diff --git a/performance-test/README b/performance-test/README
index 1481660..d1fb6de 100644
--- a/performance-test/README
+++ b/performance-test/README
@@ -36,8 +36,8 @@ for a list of mirrors.
 Running tests
 -

-The easiest way to run performance tests is to say "make perf-test", (or
-simply run the notmuch-perf-test script). Either command will run all
+The easiest way to run performance tests is to say "make time-test", (or
+simply run the notmuch-time-test script). Either command will run all
 available performance tests.

 Alternately, you can run a specific subset of tests by simply invoking
@@ -51,7 +51,7 @@ Each test script supports the following arguments
 Writing tests
 -

-Have a look at "01-dump-restore" for an example. Sourcing
+Have a look at "T01-dump-restore" for an example. Sourcing
 "perf-test-lib.sh" is mandatory.  Utility functions include

 - 'add_email_corpus' unpacks a set of messages and adds them to the database.
@@ -65,4 +65,5 @@ Have a look at "01-dump-restore" for an example. Sourcing

 Scripts are run in the order specified in notmuch-perf-test. In the
 future this order might be chosen automatically so please follow the
-convention of starting the name with two digits to specify the order.
+convention of starting the name with 'T' followed by two digits to
+specify the order.
diff --git a/performance-test/T00-new b/performance-test/T00-new
new file mode 100755
index 000..553bb8b
--- /dev/null
+++ b/performance-test/T00-new
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+test_description='notmuch new'
+
+. ./perf-test-lib.sh
+
+uncache_database
+
+time_start
+
+for i in $(seq 2 6); do
+time_run "notmuch new #$i" 'notmuch new'
+done
+
+time_done
diff --git a/performance-test/T01-dump-restore 
b/performance-test/T01-dump-restore
new file mode 100755
index 000..b2ff940
--- /dev/null
+++ 

[PATCH 1/4] perf-test: remove redunant "initial notmuch new"

2012-12-16 Thread da...@tethera.net
From: David Bremner 

The initial notmuch-new and caching are now done automatically by
time_start
---
 performance-test/00-new |4 
 1 file changed, 4 deletions(-)

diff --git a/performance-test/00-new b/performance-test/00-new
index 6f0b50c..553bb8b 100755
--- a/performance-test/00-new
+++ b/performance-test/00-new
@@ -8,10 +8,6 @@ uncache_database

 time_start

-time_run 'initial notmuch new' 'notmuch new'
-
-cache_database
-
 for i in $(seq 2 6); do
 time_run "notmuch new #$i" 'notmuch new'
 done
-- 
1.7.10.4



memory leak tests

2012-12-16 Thread da...@tethera.net
These are slightly rough around the edges, but I think they are useful.
They already helped me track down a memory leak in notmuch new
(id:1355234087-6886-1-git-send-email-david at tethera.net, 
id:1355196820-29734-1-git-send-email-david at tethera.net)

They also would have caught the restore leak Jani pointed out in
parse_sup_line, although that is a bit of hindsight since I didn't
write the obvious test until Jani pointed out the leak.


[PATCH v2 3/4] test: notmuch search --format=text0

2012-12-16 Thread Austin Clements
On Sun, 09 Dec 2012, Jani Nikula  wrote:
> ---
>  test/text |   29 +
>  1 file changed, 29 insertions(+)
>
> diff --git a/test/text b/test/text
> index 428c89b..e003a66 100755
> --- a/test/text
> +++ b/test/text
> @@ -52,4 +52,33 @@ output=$(notmuch search --format=text 
> "t?xt-search-m?ssage" | notmuch_search_s
>  test_expect_equal "$output" \
>  "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
> text-search-utf8-body-s?bj?ct (inbox unread)"
>  
> +add_email_corpus
> +
> +test_begin_subtest "Search message tags: text0"
> +cat < EXPECTED.$test_count

Other tests use just OUTPUT and EXPECTED.  Why the $test_count?  Is
there a technical reason for it?

> +attachment inbox signed unread
> +EOF
> +notmuch search --format=text0 --output=tags '*' | xargs -0 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for threads"
> +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count

I think it would be worth "strengthening" these tests as Mark pointed
out.  It would be easy to accidentally include a literal \n in the
output instead of calling the separator method, and this test wouldn't
catch that.  I think Mark's suggestion with tr is pretty good, since it
directly disambiguates \0 and \n in the output, while producing a
reasonable diff if things do go wrong.

> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for messages"
> +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for files"
> +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for tags"
> +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
>  test_done
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v5b] show: indicate length, encoding of omitted body content

2012-12-16 Thread Peter Wang
If a leaf part's body content is omitted, return the encoded length and
transfer encoding in --format=json output.  This information may be used
by the consumer, e.g. to decide whether to download a large attachment
over a slow link.

Returning the _encoded_ content length is more efficient than returning
the _decoded_ content length.  Returning the transfer encoding allows
the consumer to estimate the decoded content length.
---
 devel/schemata |  9 -
 notmuch-show.c | 16 ++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index d1ab983..7d167be 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -75,7 +75,14 @@ part = {
 # A leaf part's body content is optional, but may be included if
 # it can be correctly encoded as a string.  Consumers should use
 # this in preference to fetching the part content separately.
-content?:   string
+content?:   string,
+# If a leaf part's body content is not included, the length of
+# the encoded content (in bytes) may be given instead.
+content-length?: int,
+# If a leaf part's body content is not included, its transfer encoding
+# may be given.  Using this and the encoded content length, it is
+# possible for the consumer to estimate the decoded content length.
+content-transfer-encoding?: string
 }

 # The headers of a message or part (format_headers_sprinter with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index 6a9278c..2d59865 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
 }

 static void
-format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta, 
GMimePart *part)
 {
 const char *content_charset = g_mime_object_get_content_type_parameter 
(meta, "charset");
+const char *cte = g_mime_object_get_header (meta, 
"content-transfer-encoding");
+GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part);
+GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+ssize_t content_length = g_mime_stream_length (stream);

 if (content_charset != NULL) {
sp->map_key (sp, "content-charset");
sp->string (sp, content_charset);
 }
+if (cte != NULL) {
+   sp->map_key (sp, "content-transfer-encoding");
+   sp->string (sp, cte);
+}
+if (content_length >= 0) {
+   sp->map_key (sp, "content-length");
+   sp->integer (sp, content_length);
+}
 }

 void
@@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
} else {
-   format_omitted_part_meta_sprinter (sp, meta);
+   format_omitted_part_meta_sprinter (sp, meta, GMIME_PART 
(node->part));
}
 } else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
-- 
1.7.12.1



[PATCH v5b] show: indicate charset for all omitted parts

2012-12-16 Thread Peter Wang
Write a "charset" field for all omitted parts for which it is applicable,
not only text/html parts. Factor out the code to a separate function.
It will be extended with more fields next.
---
 notmuch-show.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..6a9278c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
 return NOTMUCH_STATUS_SUCCESS;
 }

+static void
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+{
+const char *content_charset = g_mime_object_get_content_type_parameter 
(meta, "charset");
+
+if (content_charset != NULL) {
+   sp->map_key (sp, "content-charset");
+   sp->string (sp, content_charset);
+}
+}
+
 void
 format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
  notmuch_bool_t first, notmuch_bool_t output_body)
@@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
 * makes charset decoding the responsibility on the caller, we
 * report the charset for text/html parts.
 */
-   if (g_mime_content_type_is_type (content_type, "text", "html")) {
-   const char *content_charset = 
g_mime_object_get_content_type_parameter (meta, "charset");
-
-   if (content_charset != NULL) {
-   sp->map_key (sp, "content-charset");
-   sp->string (sp, content_charset);
-   }
-   } else if (g_mime_content_type_is_type (content_type, "text", "*")) {
+   if (g_mime_content_type_is_type (content_type, "text", "*") &&
+   ! g_mime_content_type_is_type (content_type, "text", "html"))
+   {
GMimeStream *stream_memory = g_mime_stream_mem_new ();
GByteArray *part_content;
show_text_part_content (node->part, stream_memory, 0);
@@ -692,6 +698,8 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, 
mime_node_t *node,
sp->map_key (sp, "content");
sp->string_len (sp, (char *) part_content->data, part_content->len);
g_object_unref (stream_memory);
+   } else {
+   format_omitted_part_meta_sprinter (sp, meta);
}
 } else if (GMIME_IS_MULTIPART (node->part)) {
sp->map_key (sp, "content");
-- 
1.7.12.1



[PATCH] contrib: pick: close message pane when quitting from show in the message pane

2012-12-16 Thread Mark Walters
We add a hook to the show buffer in the message window to close the
message window when that buffer quits.  It checks that the
message-window is still displaying the show-message buffer and then
closes it.
---

This is (probably) a rather better version than the previous
attempt. It uses hooks rather than redefining notmuch-kill-this-buffer
and it avoids errors if the user has made the message pane the whole
frame.

Best wishes

Mark


 contrib/notmuch-pick/notmuch-pick.el |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el 
b/contrib/notmuch-pick/notmuch-pick.el
index 043e9e7..8383589 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -160,6 +160,9 @@
 (defvar notmuch-pick-message-window nil)
 (make-variable-buffer-local 'notmuch-pick-message-window)
 (put 'notmuch-pick-message-window 'permanent-local t)
+(defvar notmuch-show-message-window nil)
+(make-variable-buffer-local 'notmuch-show-message-window)
+(put 'notmuch-show-message-window 'permanent-local t)
 (defvar notmuch-pick-message-buffer nil)
 (make-variable-buffer-local 'notmuch-pick-message-buffer-name)
 (put 'notmuch-pick-message-buffer-name 'permanent-local t)
@@ -389,6 +392,16 @@ Does NOT change the database."
 (notmuch-prettify-subject (notmuch-search-find-subject)))
   (notmuch-pick-show-match-message-with-wait))

+(defun notmuch-pick-message-window-kill-hook ()
+  (let ((buffer (current-buffer)))
+(when (and (window-live-p notmuch-show-message-window)
+  (eq (window-buffer notmuch-show-message-window) buffer))
+  ;; We do not want an error if this is the sole window in the
+  ;; frame and I do not know how to test for that in emacs pre
+  ;; 24. Hence we just ignore-errors.
+  (ignore-errors
+   (delete-window notmuch-show-message-window)
+
 (defun notmuch-pick-show-message ()
   "Show the current message (in split-pane)."
   (interactive)
@@ -406,6 +419,11 @@ Does NOT change the database."
(let ((notmuch-show-indent-messages-width 0))
  (setq current-prefix-arg '(4))
  (setq buffer (notmuch-show id nil nil nil
+  ;; We need the `let' as notmuch-pick-message-window is buffer local.
+  (let ((window notmuch-pick-message-window))
+   (with-current-buffer buffer
+ (setq notmuch-show-message-window window)
+ (add-hook 'kill-buffer-hook 'notmuch-pick-message-window-kill-hook)))
   (notmuch-pick-tag-update-display (list "-unread"))
   (setq notmuch-pick-message-buffer buffer

-- 
1.7.9.1



[PATCH v2 3/4] test: notmuch search --format=text0

2012-12-16 Thread Mark Walters

On Sun, 09 Dec 2012, Jani Nikula  wrote:
> ---
>  test/text |   29 +
>  1 file changed, 29 insertions(+)
>
> diff --git a/test/text b/test/text
> index 428c89b..e003a66 100755
> --- a/test/text
> +++ b/test/text
> @@ -52,4 +52,33 @@ output=$(notmuch search --format=text 
> "t?xt-search-m?ssage" | notmuch_search_s
>  test_expect_equal "$output" \
>  "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
> text-search-utf8-body-s?bj?ct (inbox unread)"
>  
> +add_email_corpus
> +
> +test_begin_subtest "Search message tags: text0"
> +cat < EXPECTED.$test_count
> +attachment inbox signed unread
> +EOF
> +notmuch search --format=text0 --output=tags '*' | xargs -0 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for threads"
> +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count

Hi

These xargs -0 -L1 tests almost pass with format=text (no zero) passed:
the output only differs in one newline at the end. Would it be worth
strengthening the test at all? I don't have any good suggestion but
replacing the xargs with
tr '\n\0' ' \n'
seemed to give clearly different output in the two cases (and the test
passes as it stands).

OTOH maybe the test before is sufficient in that respect.

Best wishes

Mark


> +
> +test_begin_subtest "Compare text vs. text0 for messages"
> +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
> > EXPECTED.$test_count
> +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for files"
> +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
> +test_begin_subtest "Compare text vs. text0 for tags"
> +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > 
> EXPECTED.$test_count
> +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
> notmuch_search_sanitize > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
>  test_done
> -- 
> 1.7.10.4


[PATCH v5 0/4] indicate length of omitted body content

2012-12-16 Thread Mark Walters

The b versions of this look good to me.

Best wishes

Mark

On Sat, 15 Dec 2012, David Bremner  wrote:
> Peter Wang  writes:
>
>> On Sat, 15 Dec 2012 08:45:33 +, Mark Walters > gmail.com> wrote:
>>> 
>>> This version looks good to me with one very minor comments:
>>> 
>>> Perhaps the name format-omitted-part-meta should include sprinter (eg
>>> format-omitted-part-meta-sprinter)?
>>
>> Seems so. I'll roll up another series if that's necessary.
>
> Maybe just post the one amended patch as a reply to the patch it's
> amending?
>
> d
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 0/7] Structed output versioning support

2012-12-16 Thread Mark Walters

LGTM +1

Best wishes

Mark

On Sun, 16 Dec 2012, Austin Clements  wrote:
> This series obsoletes [0] and must be applied on top of the Emacs CLI
> error handling series [1].  This is much simpler than the original
> series because it no longer includes the Emacs CLI error handling.
> This also switches to a CLI-wide minimum version instead of a
> per-command version, prints a warning if an old but still supported
> version is requested to ease client maintenance, and renames
> --use-schema to --format-version (which parallels --format better and
> simplifies error text).
>
> [0] id:1354416002-3557-1-git-send-email-amdragon at mit.edu
> [1] id:1355601860-30121-1-git-send-email-amdragon at mit.edu


[PATCH v2 0/7] Improve Emacs CLI error handling

2012-12-16 Thread Tomi Ollila
On Sat, Dec 15 2012, Austin Clements  wrote:

> This obsoletes id:1355548513-10085-1-git-send-email-amdragon at mit.edu
> and fixes the things Mark and Tomi commented on.  The interdiff is
> below.

Ok, new error messages appear at the end. Good.

+1

Tomi

>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index cf61635..8f84087 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -326,10 +326,12 @@ the user dismisses it."
>  (with-current-buffer buf
>(view-mode-enter nil #'kill-buffer)
>(let ((inhibit-read-only t))
> + (goto-char (point-max))
> + (unless (bobp)
> +   (insert "\n"))
>   (insert msg)
>   (unless (bolp)
> -   (insert "\n"))
> - (goto-char (point-min
> +   (insert "\n"
>  (pop-to-buffer buf)))
>  


[Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries

2012-12-16 Thread Jani Nikula
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner 
>
> The query is split into tokens, with ' ' and ':' as delimiters.  Any
> token containing some hex-escaped character is quoted according to
> Xapian rules.  This maps id:foo%20%22bar to id:"foo ""bar".
> This intentionally does not quote prefixes, so they still work as prefixes.
> ---
>  tag-util.c |   50 ++
>  1 file changed, 50 insertions(+)
>
> diff --git a/tag-util.c b/tag-util.c
> index f89669a..e1181f8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove)
>  return NULL;
>  }
>  
> +static tag_parse_status_t
> +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error,
> + char **query_string)
> +{
> +char *tok = encoded;
> +size_t tok_len = 0;
> +char *buf = NULL;
> +size_t buf_len = 0;
> +tag_parse_status_t ret = TAG_PARSE_SUCCESS;
> +
> +*query_string = talloc_strdup (ctx, "");
> +
> +while (*query_string &&
> +(tok = strtok_len (tok + tok_len, ": ", _len)) != NULL) {

strtok_len() will eat all the leading delimiters at each call, and will
not return a zero-length token if you have multiple consecutive
delimiters. Which means you may end up losing stuff here. Whether that
matters or not I'm too tired to tell...

BR,
Jani.


> + char delim = tok[tok_len];
> +
> + *(tok + tok_len++) = '\0';
> +
> + if (strcspn (tok, "%") < tok_len - 1) {
> + /* something to decode */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> +   "hex decoding of token '%s' failed", tok);
> + goto DONE;
> + }
> +
> + if (double_quote_str (ctx, tok, , _len)) {
> + ret = line_error (TAG_PARSE_OUT_OF_MEMORY,
> +   line_for_error, "aborting");
> + goto DONE;
> + }
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", buf, delim);
> +
> + } else {
> + /* This is not just an optimization, but used to preserve
> +  * prefixes like id:, which cannot be quoted.
> +  */
> + *query_string = talloc_asprintf_append_buffer (
> + *query_string, "%s%c", tok, delim);
> + }
> +
> +}
> +
> +  DONE:
> +if (ret != TAG_PARSE_SUCCESS && *query_string)
> + talloc_free (*query_string);
> +return ret;
> +}
> +
>  tag_parse_status_t
>  parse_tag_line (void *ctx, char *line,
>   tag_op_flag_t flags,
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag"

2012-12-16 Thread Jani Nikula
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: Jani Nikula 
>
> Add support for batch tagging operations through stdin to "notmuch
> tag". This can be enabled with the new --batch command line option to
> "notmuch tag". The input must consist of lines of the format:
>
> +|- [...] [--]  [...]
>
> Each line is interpreted similarly to "notmuch tag" command line
> arguments. The delimiter is one or more spaces ' '. Any characters in
>  and  MAY be hex encoded with %NN where NN is the
> hexadecimal value of the character. Any ' ' and '%' characters in
>  and  MUST be hex encoded (using %20 and %25,
> respectively). Any characters that are not part of  or
>  MUST NOT be hex encoded.
>
> Leading and trailing space ' ' is ignored. Empty lines and lines
> beginning with '#' are ignored.
>
> Signed-off-by: Jani Nikula 
>
> Hacked-like-crazy-by: David Bremner 
> ---
>  notmuch-tag.c |   88 
> +++--
>  1 file changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 13f2268..a81d911 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -130,6 +130,43 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
> char *query_string,
>  return interrupted;
>  }
>  
> +static int
> +tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags,
> +   FILE *input)
> +{
> +char *line = NULL;
> +char *query_string = NULL;
> +size_t line_size = 0;
> +ssize_t line_len;
> +int ret = 0;
> +tag_op_list_t *tag_ops;
> +
> +tag_ops = tag_op_list_create (ctx);
> +if (tag_ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> +}
> +
> +while ((line_len = getline (, _size, input)) != -1 &&
> +! interrupted) {
> +
> + ret = parse_tag_line (ctx, line, TAG_FLAG_NONE,
> +   _string, tag_ops);
> +
> + if (ret > 0)
> + continue;
> +
> + if (ret < 0 || tag_query (ctx, notmuch, query_string,
> +   tag_ops, flags))
> + break;

Hey, your changelog in the cover letter says you fixed the tag_query
return value propagation, but it's not here?

BR,
Jani.


> +}
> +
> +if (line)
> + free (line);
> +
> +return ret;
> +}
> +
>  int
>  notmuch_tag_command (void *ctx, int argc, char *argv[])
>  {
> @@ -139,6 +176,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  notmuch_database_t *notmuch;
>  struct sigaction action;
>  tag_op_flag_t tag_flags = TAG_FLAG_NONE;
> +notmuch_bool_t batch = FALSE;
> +FILE *input = stdin;
> +char *input_file_name = NULL;
> +int opt_index;
>  int ret = 0;
>  
>  /* Setup our handler for SIGINT */
> @@ -148,15 +189,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  action.sa_flags = SA_RESTART;
>  sigaction (SIGINT, , NULL);
>  
> -tag_ops = tag_op_list_create (ctx);
> -if (tag_ops == NULL) {
> - fprintf (stderr, "Out of memory.\n");
> +notmuch_opt_desc_t options[] = {
> + { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 },
> + { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 },
> + { 0, 0, 0, 0, 0 }
> +};
> +
> +opt_index = parse_arguments (argc, argv, options, 1);
> +if (opt_index < 0)
>   return 1;
> +
> +if (input_file_name) {
> + batch = TRUE;
> + input = fopen (input_file_name, "r");
> + if (input == NULL) {
> + fprintf (stderr, "Error opening %s for reading: %s\n",
> +  input_file_name, strerror (errno));
> + return 1;
> + }
>  }
>  
> -if (parse_tag_command_line (ctx, argc - 1, argv + 1,
> - _string, tag_ops))
> - return 1;
> +if (batch) {
> + if (opt_index != argc) {
> + fprintf (stderr, "Can't specify both cmdline and stdin!\n");
> + return 1;
> + }
> +} else {
> +
> + tag_ops = tag_op_list_create (ctx);
> + if (tag_ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> +
> + if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
> + _string, tag_ops))
> + return 1;
> +}
>  
>  config = notmuch_config_open (ctx, NULL, NULL);
>  if (config == NULL)
> @@ -169,9 +238,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  if (notmuch_config_get_maildir_synchronize_flags (config))
>   tag_flags |= TAG_FLAG_MAILDIR_SYNC;
>  
> -ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
> +if (batch)
> + ret = tag_file (ctx, notmuch, tag_flags, input);
> +else
> + ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags);
>  
>  notmuch_database_destroy (notmuch);
>  
> -return ret;
> +return ret || interrupted;
>  }
> -- 
> 1.7.10.4


[PATCH] fixup for hex encoding desription in notmuch-tag.1

2012-12-16 Thread Jani Nikula
On Sat, 15 Dec 2012, david at tethera.net wrote:
> From: David Bremner 
>
> ---
> and here is the updated description.
>
>  man/man1/notmuch-tag.1 |   19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index ac2c8d7..a203f53 100644
> --- a/man/man1/notmuch-tag.1
> +++ b/man/man1/notmuch-tag.1
> @@ -67,13 +67,22 @@ The input must consist of lines of the format:
>  Each line is interpreted similarly to
>  .B notmuch tag
>  command line arguments. The delimiter is one or more spaces ' '. Any
> -characters in  and 
> +characters in
> +.RI < tag >
> +and
> +.RI < search-term >
>  .B may
> -be hex encoded with %NN where NN is the hexadecimal value of the
> -character. Any ' ' and '%' characters in  and 
> +be hex-encoded with %NN where NN is the hexadecimal value of the
> +character (more precisely with %NN[%MM...] where NN, MM, etc... are
> +the hexadecimal values of the bytes in the UTF-8 encoding of
> +the character).  Any characters in  and  not
> +matching the regex
> +.B [A-Za-z0-9@=.,_+-]

And this actually means you can't have "id:foo" there, as : needs to be
encoded. Not user friendly at all. If it should really mean : is okay
un-encoded as prefix delimiter but not otherwise... it gets a bit messy
to document.

BR,
Jani.

>  .B must
> -be hex encoded (using %20 and %25, respectively). Any characters that
> -are not part of  or 
> +be hex-encoded. Any characters that are not part of
> +.RI  < tag >
> +or
> +.RI < search-term >
>  .B must not
>  be hex encoded.
>  
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


[Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser

2012-12-16 Thread Jani Nikula
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner 
>
> We are able to detect more errors by looking at the string before it
> is hex-decoded. We also need this to avoid the query quoting for more
> general queries (to be written) that will mess up raw message-ids.
> ---
>  notmuch-restore.c |   18 +-
>  tag-util.c|   26 --
>  tag-util.h|5 -
>  test/dump-restore |5 ++---
>  4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 40596a8..112f2f3 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc, 
> char *argv[])
>   if (input_format == DUMP_FORMAT_SUP) {
>   ret = parse_sup_line (ctx, line, _string, tag_ops);
>   } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | 
> TAG_FLAG_ID_ONLY,
> _string, tag_ops);

I realize that we've screwed up a bit here already in the restore
series. parse_sup_line() allocates query_string for each input line, but
doesn't free it during parsing. parse_tag_line() sets query_string to
point to the query string within line. And now it gets worse when
query_string is allocated vs. set to point to another buffer depending
on TAG_FLAG_ID_ONLY, so it's not an easy interface to use.

> -
> - if (ret == 0) {
> - if (strncmp ("id:", query_string, 3) != 0) {
> - fprintf (stderr, "Warning: unsupported query: %s\n", 
> query_string);
> - continue;
> - }
> - /* delete id: from front of string; tag_message
> -  * expects a raw message-id.
> -  *
> -  * XXX: Note that query string id:foo and bar will be
> -  * interpreted as a message id "foo and bar". This
> -  * should eventually be fixed to give a better error
> -  * message.
> -  */
> - query_string = query_string + 3;
> - }



>   }
>  
>   if (ret > 0)
> diff --git a/tag-util.c b/tag-util.c
> index e1181f8..8fea76c 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line,
>  }
>  
>  /* tok now points to the query string */
> -if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> -   "hex decoding of query %s failed", tok);
> - goto DONE;
> +if (flags & TAG_FLAG_ID_ONLY) {
> + /* this is under the assumption that any whitespace in the
> +  * message-id must be hex-encoded. The check is probably not
> +  * perfect for exotic unicode whitespace; as fallback the
> +  * search for strange message-ids will fail */
> + if ((strncmp ("id:", tok, 3) != 0) ||

The current wording is, "Any characters in  and  MAY
be hex encoded with %NN...", but the above no longer matches that, as
"id:" must not be encoded. In the interest of keeping the documentation
concise, I think you should move the above check after
hex_decode_inplace(), but keep the below check here. That should do it,
right?

> + (strcspn (tok, " \t") < strlen (tok))) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> +   "query '%s' is not 'id:'", tok);
> + goto DONE;
> + }
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error,
> +   "hex decoding of query %s failed", tok);
> + goto DONE;
> + }
> + /* skip 'id:' */
> + *query_string = tok + 3;
> +} else {
> + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string);
>  }

It's not pretty that query_string gets allocated or not depending on a
flag that doesn't seem to have anything to do with that. I don't have a
good suggestion now, though, and I'd like to keep the restore path like
it is, without allocation.

>  
> -*query_string = tok;
> -
>DONE:
>  talloc_free (line_for_error);
>  return ret;
> diff --git a/tag-util.h b/tag-util.h
> index 2889736..7674051 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -26,7 +26,10 @@ typedef enum {
>  /* Accept strange tags that might be user error;
>   * intended for use by notmuch-restore.
>   */
> -TAG_FLAG_BE_GENEROUS = (1 << 3)
> +TAG_FLAG_BE_GENEROUS = (1 << 3),
> +
> +/* Query consists of a single id:$message-id */
> +TAG_FLAG_ID_ONLY = (1 << 4)
>  
>  } tag_op_flag_t;
>  
> diff --git a/test/dump-restore b/test/dump-restore
> index 6a989b6..eb7933a 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -199,19 +199,18 @@ a
>  # the next non-comment line should report an an empty tag error for
>  # batch tagging, but not for restore
> 

[Patch v7 04/14] notmuch-tag: factor out double quoting routine

2012-12-16 Thread Jani Nikula
On Fri, 14 Dec 2012, david at tethera.net wrote:
> From: David Bremner 
>
> This could live in tag-util as well, but it is really nothing specific
> to tags (although the conventions are arguable specific to Xapian).
>
> The API is changed from "caller-allocates" to "readline-like". The scan for
> max tag length is pushed down into the double quoting routine.
> ---
>  notmuch-tag.c  |   50 --
>  util/string-util.c |   34 ++
>  util/string-util.h |8 
>  3 files changed, 58 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 0965ee7..13f2268 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -20,6 +20,7 @@
>  
>  #include "notmuch-client.h"
>  #include "tag-util.h"
> +#include "string-util.h"
>  
>  static volatile sig_atomic_t interrupted;
>  
> @@ -37,25 +38,6 @@ handle_sigint (unused (int sig))
>  }
>  
>  static char *
> -_escape_tag (char *buf, const char *tag)
> -{
> -const char *in = tag;
> -char *out = buf;
> -
> -/* Boolean terms surrounded by double quotes can contain any
> - * character.  Double quotes are quoted by doubling them. */
> -*out++ = '"';
> -while (*in) {
> - if (*in == '"')
> - *out++ = '"';
> - *out++ = *in++;
> -}
> -*out++ = '"';
> -*out = 0;
> -return buf;
> -}
> -
> -static char *
>  _optimize_tag_query (void *ctx, const char *orig_query_string,
>const tag_op_list_t *list)
>  {
> @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char 
> *orig_query_string,
>   * parenthesize and the exclusion part of the query must not use
>   * the '-' operator (though the NOT operator is fine). */
>  
> -char *escaped, *query_string;
> +char *escaped = NULL;
> +size_t escaped_len = 0;
> +char *query_string;
>  const char *join = "";
>  size_t i;
> -unsigned int max_tag_len = 0;
>  
>  /* Don't optimize if there are no tag changes. */
>  if (tag_op_list_size (list) == 0)
>   return talloc_strdup (ctx, orig_query_string);
>  
> -/* Allocate a buffer for escaping tags.  This is large enough to
> - * hold a fully escaped tag with every character doubled plus
> - * enclosing quotes and a NUL. */
> -for (i = 0; i < tag_op_list_size (list); i++)
> - if (strlen (tag_op_list_tag (list, i)) > max_tag_len)
> - max_tag_len = strlen (tag_op_list_tag (list, i));
> -
> -escaped = talloc_array (ctx, char, max_tag_len * 2 + 3);
> -if (! escaped)
> - return NULL;
> -
>  /* Build the new query string */
>  if (strcmp (orig_query_string, "*") == 0)
>   query_string = talloc_strdup (ctx, "(");
>  else
>   query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string);
>  
> +
> +/* Boolean terms surrounded by double quotes can contain any
> + * character.  Double quotes are quoted by doubling them. */
> +
>  for (i = 0; i < tag_op_list_size (list) && query_string; i++) {
> + double_quote_str (ctx,
> +   tag_op_list_tag (list, i),
> +   , _len);

Check return value?

> +
>   query_string = talloc_asprintf_append_buffer (
>   query_string, "%s%stag:%s", join,
>   tag_op_list_isremove (list, i) ? "" : "not ",
> - _escape_tag (escaped, tag_op_list_tag (list, i)));
> + escaped);
>   join = " or ";
>  }
>  
>  if (query_string)
>   query_string = talloc_strdup_append_buffer (query_string, ")");
>  
> -talloc_free (escaped);
> +if (escaped)
> + talloc_free (escaped);
> +
>  return query_string;
>  }
>  
> diff --git a/util/string-util.c b/util/string-util.c
> index 44f8cd3..ea7c25b 100644
> --- a/util/string-util.c
> +++ b/util/string-util.c
> @@ -20,6 +20,7 @@
>  
>  
>  #include "string-util.h"
> +#include "talloc.h"
>  
>  char *
>  strtok_len (char *s, const char *delim, size_t *len)
> @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len)
>  
>  return *len ? s : NULL;
>  }
> +
> +
> +int
> +double_quote_str (void *ctx, const char *str,
> +   char **buf, size_t *len)
> +{
> +const char *in;
> +char *out;
> +size_t needed = 3;
> +
> +for (in = str; *in; in++)
> + needed += (*in == '"') ? 2 : 1;
> +
> +if (needed > *len)
> + *buf = talloc_realloc (ctx, *buf, char, 2*needed);

You fail to set *len to 2*needed, leading to doing realloc every time.

Also, I think you should follow the getline pattern like you did in
hex_encode: if *buf == NULL, the input value of *len is ignored.

BR,
Jani.

> +
> +if (! *buf)
> + return 1;
> +
> +out = *buf;
> +
> +*out++ = '"';
> +in = str;
> +while (*in) {
> + if (*in == '"')
> + *out++ = '"';
> + *out++ = *in++;
> +}
> +*out++ = '"';
> +*out = 0;
> +
> +return 0;
> +}
> diff --git a/util/string-util.h 

Re: [PATCH v5 0/4] indicate length of omitted body content

2012-12-16 Thread Mark Walters

The b versions of this look good to me.

Best wishes

Mark

On Sat, 15 Dec 2012, David Bremner da...@tethera.net wrote:
 Peter Wang noval...@gmail.com writes:

 On Sat, 15 Dec 2012 08:45:33 +, Mark Walters markwalters1...@gmail.com 
 wrote:
 
 This version looks good to me with one very minor comments:
 
 Perhaps the name format-omitted-part-meta should include sprinter (eg
 format-omitted-part-meta-sprinter)?

 Seems so. I'll roll up another series if that's necessary.

 Maybe just post the one amended patch as a reply to the patch it's
 amending?

 d
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] contrib: pick: close message pane when quitting from show in the message pane

2012-12-16 Thread Mark Walters
We add a hook to the show buffer in the message window to close the
message window when that buffer quits.  It checks that the
message-window is still displaying the show-message buffer and then
closes it.
---

This is (probably) a rather better version than the previous
attempt. It uses hooks rather than redefining notmuch-kill-this-buffer
and it avoids errors if the user has made the message pane the whole
frame.

Best wishes

Mark


 contrib/notmuch-pick/notmuch-pick.el |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el 
b/contrib/notmuch-pick/notmuch-pick.el
index 043e9e7..8383589 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -160,6 +160,9 @@
 (defvar notmuch-pick-message-window nil)
 (make-variable-buffer-local 'notmuch-pick-message-window)
 (put 'notmuch-pick-message-window 'permanent-local t)
+(defvar notmuch-show-message-window nil)
+(make-variable-buffer-local 'notmuch-show-message-window)
+(put 'notmuch-show-message-window 'permanent-local t)
 (defvar notmuch-pick-message-buffer nil)
 (make-variable-buffer-local 'notmuch-pick-message-buffer-name)
 (put 'notmuch-pick-message-buffer-name 'permanent-local t)
@@ -389,6 +392,16 @@ Does NOT change the database.
 (notmuch-prettify-subject (notmuch-search-find-subject)))
   (notmuch-pick-show-match-message-with-wait))
 
+(defun notmuch-pick-message-window-kill-hook ()
+  (let ((buffer (current-buffer)))
+(when (and (window-live-p notmuch-show-message-window)
+  (eq (window-buffer notmuch-show-message-window) buffer))
+  ;; We do not want an error if this is the sole window in the
+  ;; frame and I do not know how to test for that in emacs pre
+  ;; 24. Hence we just ignore-errors.
+  (ignore-errors
+   (delete-window notmuch-show-message-window)
+
 (defun notmuch-pick-show-message ()
   Show the current message (in split-pane).
   (interactive)
@@ -406,6 +419,11 @@ Does NOT change the database.
(let ((notmuch-show-indent-messages-width 0))
  (setq current-prefix-arg '(4))
  (setq buffer (notmuch-show id nil nil nil
+  ;; We need the `let' as notmuch-pick-message-window is buffer local.
+  (let ((window notmuch-pick-message-window))
+   (with-current-buffer buffer
+ (setq notmuch-show-message-window window)
+ (add-hook 'kill-buffer-hook 'notmuch-pick-message-window-kill-hook)))
   (notmuch-pick-tag-update-display (list -unread))
   (setq notmuch-pick-message-buffer buffer
 
-- 
1.7.9.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 3/4] test: notmuch search --format=text0

2012-12-16 Thread Austin Clements
On Sun, 09 Dec 2012, Jani Nikula j...@nikula.org wrote:
 ---
  test/text |   29 +
  1 file changed, 29 insertions(+)

 diff --git a/test/text b/test/text
 index 428c89b..e003a66 100755
 --- a/test/text
 +++ b/test/text
 @@ -52,4 +52,33 @@ output=$(notmuch search --format=text 
 tëxt-search-méssage | notmuch_search_s
  test_expect_equal $output \
  thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
 text-search-utf8-body-sübjéct (inbox unread)
  
 +add_email_corpus
 +
 +test_begin_subtest Search message tags: text0
 +cat EOF  EXPECTED.$test_count

Other tests use just OUTPUT and EXPECTED.  Why the $test_count?  Is
there a technical reason for it?

 +attachment inbox signed unread
 +EOF
 +notmuch search --format=text0 --output=tags '*' | xargs -0 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
 +test_begin_subtest Compare text vs. text0 for threads
 +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
  EXPECTED.$test_count
 +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count

I think it would be worth strengthening these tests as Mark pointed
out.  It would be easy to accidentally include a literal \n in the
output instead of calling the separator method, and this test wouldn't
catch that.  I think Mark's suggestion with tr is pretty good, since it
directly disambiguates \0 and \n in the output, while producing a
reasonable diff if things do go wrong.

 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
 +test_begin_subtest Compare text vs. text0 for messages
 +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
  EXPECTED.$test_count
 +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
 +test_begin_subtest Compare text vs. text0 for files
 +notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
 EXPECTED.$test_count
 +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
 +test_begin_subtest Compare text vs. text0 for tags
 +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
 EXPECTED.$test_count
 +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
  test_done
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


memory leak tests

2012-12-16 Thread david
These are slightly rough around the edges, but I think they are useful.
They already helped me track down a memory leak in notmuch new
(id:1355234087-6886-1-git-send-email-da...@tethera.net, 
id:1355196820-29734-1-git-send-email-da...@tethera.net)

They also would have caught the restore leak Jani pointed out in
parse_sup_line, although that is a bit of hindsight since I didn't
write the obvious test until Jani pointed out the leak.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/4] perf-test: remove redunant initial notmuch new

2012-12-16 Thread david
From: David Bremner brem...@debian.org

The initial notmuch-new and caching are now done automatically by
time_start
---
 performance-test/00-new |4 
 1 file changed, 4 deletions(-)

diff --git a/performance-test/00-new b/performance-test/00-new
index 6f0b50c..553bb8b 100755
--- a/performance-test/00-new
+++ b/performance-test/00-new
@@ -8,10 +8,6 @@ uncache_database
 
 time_start
 
-time_run 'initial notmuch new' 'notmuch new'
-
-cache_database
-
 for i in $(seq 2 6); do
 time_run notmuch new #$i 'notmuch new'
 done
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/4] perf-test: initial version of memory test infrastructure.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

The idea is run some code under valgrind --leak-check=full and report
a summary, leaving the user to peruse the log file if they want.

We go to some lengths to preserve the log files from accidental
overwriting; the full corpus takes about 3 hours to run under valgrind
on my machine.

The naming of the log directories is probably overkill; I find it nice
to have them sequenced by time. Arguably the mktemp is then overkill,
but I know people will be nervous if it looks like timestamps are
being used for uniqueness.

One new test is included, to check notmuch new for memory leaks.
---
 performance-test/.gitignore   |1 +
 performance-test/M00-new  |   14 +
 performance-test/Makefile.local   |   17 +--
 performance-test/README   |   57 +
 performance-test/perf-test-lib.sh |   55 ---
 5 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100755 performance-test/M00-new

diff --git a/performance-test/.gitignore b/performance-test/.gitignore
index 6421a9a..f3f9be4 100644
--- a/performance-test/.gitignore
+++ b/performance-test/.gitignore
@@ -1,3 +1,4 @@
 tmp.*/
+log.*/
 corpus/
 notmuch.cache.*/
diff --git a/performance-test/M00-new b/performance-test/M00-new
new file mode 100755
index 000..733e9b0
--- /dev/null
+++ b/performance-test/M00-new
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+test_description='notmuch new'
+
+. ./perf-test-lib.sh
+
+# ensure initial 'notmuch new' is run by memory_start
+uncache_database
+
+memory_start
+
+memory_run notmuch new notmuch new
+
+memory_done
diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
index 57beb44..357d800 100644
--- a/performance-test/Makefile.local
+++ b/performance-test/Makefile.local
@@ -4,14 +4,25 @@ dir := performance-test
 
 include $(dir)/version.sh
 
+# these two are just make sure dir is expanded at the right time.
+TIME_TEST_SCRIPT := ${dir}/notmuch-time-test
+MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test
+
 CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz
 TXZFILE := ${dir}/download/${CORPUS_NAME}
 SIGFILE := ${TXZFILE}.asc
-TEST_SCRIPT := ${dir}/notmuch-perf-test
 DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}
 
+perf-test: time-test memory-test
+
 time-test: setup-perf-test all
-   $(TEST_SCRIPT) $(OPTIONS)
+   @echo
+   $(TIME_TEST_SCRIPT) $(TEST_OPTIONS)
+
+memory-test: setup-perf-test all
+   @echo
+   $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS)
+
 
 .PHONY: download-corpus setup-perf-test
 
@@ -29,4 +40,4 @@ $(TXZFILE):
 download-corpus:
wget -O ${TXZFILE} ${DEFAULT_URL}
 
-CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.*
+CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus 
$(dir)/notmuch.cache.*
diff --git a/performance-test/README b/performance-test/README
index d1fb6de..7eaf5f7 100644
--- a/performance-test/README
+++ b/performance-test/README
@@ -1,3 +1,10 @@
+Performance Tests
+-
+
+This directory contains two kinds of performance tests, time tests,
+and memory tests. The former use gnu time, and the latter use
+valgrind.
+
 Pre-requisites
 --
 
@@ -5,9 +12,10 @@ In addition to having notmuch, you need:
 
 - gpg
 - gnu tar
-- gnu time
+- gnu time (for the time tests).
 - xz. Some speedup can be gotten by installing pixz, but this is
   probably only worthwhile if you are debugging the tests.
+- valgrind (for the memory tests)
 
 Getting set up to run tests:
 
@@ -36,34 +44,47 @@ for a list of mirrors.
 Running tests
 -
 
-The easiest way to run performance tests is to say make time-test, (or
-simply run the notmuch-time-test script). Either command will run all
-available performance tests.
-
-Alternately, you can run a specific subset of tests by simply invoking
-one of the executable scripts in this directory, (such as ./basic).
-Each test script supports the following arguments
+The easiest way to run performance tests is to say make perf-test.
+This will run all time and memory tests.  Be aware that the memory
+tests are quite time consuming when run on the full corpus, and that
+depending on your interests it may be more sensible to run make
+time-test or make memory-test.  You can also invoke one of the
+scripts notmuch-time-test or notmuch-memory-test or run a more
+specific subset of tests by simply invoking one of the executable
+scripts in this directory, (such as ./T00-new).  Each test script
+supports the following arguments
 
 --small / --medium / --large   Choose corpus size.
 --debugEnable debugging. In particular don't 
delete
temporary directories.
 
+When using the make targets, you can pass arguments to all test
+scripts by defining the make variable TEST_OPTIONS.
+
 Writing tests
 -
 
-Have a look at T01-dump-restore for an 

[PATCH 4/4] perf-test: add memory leak test for dump restore

2012-12-16 Thread david
From: David Bremner brem...@debian.org

In id:87vcc2q5n2@nikula.org, Jani points out a memory leak in the
current version of the sup restore code. Among other things, this test
is intended to verify a fix for that leak.
---
 performance-test/M01-dump-restore |   15 +++
 1 file changed, 15 insertions(+)
 create mode 100755 performance-test/M01-dump-restore

diff --git a/performance-test/M01-dump-restore 
b/performance-test/M01-dump-restore
new file mode 100755
index 000..be5894a
--- /dev/null
+++ b/performance-test/M01-dump-restore
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+test_description='dump and restore'
+
+. ./perf-test-lib.sh
+
+memory_start
+
+memory_run 'load nmbug tags' 'notmuch restore --accumulate 
--input=corpus.tags/nmbug.sup-dump'
+memory_run 'dump *' 'notmuch dump --output=tags.sup'
+memory_run 'restore *' 'notmuch restore --input=tags.sup'
+memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag 
--output=tags.bt'
+memory_run 'restore --format=batch-tag *' 'notmuch restore --format=batch-tag 
--input=tags.bt'
+
+memory_done
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/4] perf-test: initial version of memory test infrastructure.

2012-12-16 Thread Austin Clements
Quoth da...@tethera.net on Dec 16 at  2:23 pm:
 From: David Bremner brem...@debian.org
 
 The idea is run some code under valgrind --leak-check=full and report
 a summary, leaving the user to peruse the log file if they want.
 
 We go to some lengths to preserve the log files from accidental
 overwriting; the full corpus takes about 3 hours to run under valgrind
 on my machine.
 
 The naming of the log directories is probably overkill; I find it nice
 to have them sequenced by time. Arguably the mktemp is then overkill,
 but I know people will be nervous if it looks like timestamps are
 being used for uniqueness.
 
 One new test is included, to check notmuch new for memory leaks.
 ---
  performance-test/.gitignore   |1 +
  performance-test/M00-new  |   14 +
  performance-test/Makefile.local   |   17 +--
  performance-test/README   |   57 
 +
  performance-test/perf-test-lib.sh |   55 ---
  5 files changed, 113 insertions(+), 31 deletions(-)
  create mode 100755 performance-test/M00-new
 
 diff --git a/performance-test/.gitignore b/performance-test/.gitignore
 index 6421a9a..f3f9be4 100644
 --- a/performance-test/.gitignore
 +++ b/performance-test/.gitignore
 @@ -1,3 +1,4 @@
  tmp.*/
 +log.*/
  corpus/
  notmuch.cache.*/
 diff --git a/performance-test/M00-new b/performance-test/M00-new
 new file mode 100755
 index 000..733e9b0
 --- /dev/null
 +++ b/performance-test/M00-new
 @@ -0,0 +1,14 @@
 +#!/bin/bash
 +
 +test_description='notmuch new'
 +
 +. ./perf-test-lib.sh
 +
 +# ensure initial 'notmuch new' is run by memory_start
 +uncache_database
 +
 +memory_start
 +
 +memory_run notmuch new notmuch new

This will run notmuch new twice.  Is that intentional?  If so, it
might be worth a comment.

 +
 +memory_done
 diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
 index 57beb44..357d800 100644
 --- a/performance-test/Makefile.local
 +++ b/performance-test/Makefile.local
 @@ -4,14 +4,25 @@ dir := performance-test
  
  include $(dir)/version.sh
  
 +# these two are just make sure dir is expanded at the right time.
 +TIME_TEST_SCRIPT := ${dir}/notmuch-time-test
 +MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test

This is obviously fine, but I don't understand the comment.  There are
all sorts of places where we have to := assign to get ${dir} right
(including TXZFILE file below).  Is there something different here?

 +
  CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz
  TXZFILE := ${dir}/download/${CORPUS_NAME}
  SIGFILE := ${TXZFILE}.asc
 -TEST_SCRIPT := ${dir}/notmuch-perf-test
  DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}
  
 +perf-test: time-test memory-test
 +
  time-test: setup-perf-test all
 - $(TEST_SCRIPT) $(OPTIONS)
 + @echo
 + $(TIME_TEST_SCRIPT) $(TEST_OPTIONS)

Why not use the same OPTIONS variable name as the correctness tests?

 +
 +memory-test: setup-perf-test all
 + @echo
 + $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS)
 +
  
  .PHONY: download-corpus setup-perf-test
  
 @@ -29,4 +40,4 @@ $(TXZFILE):
  download-corpus:
   wget -O ${TXZFILE} ${DEFAULT_URL}
  
 -CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.*
 +CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus 
 $(dir)/notmuch.cache.*
 diff --git a/performance-test/README b/performance-test/README
 index d1fb6de..7eaf5f7 100644
 --- a/performance-test/README
 +++ b/performance-test/README
 @@ -1,3 +1,10 @@
 +Performance Tests
 +-
 +
 +This directory contains two kinds of performance tests, time tests,

s/performance tests,/performance tests:/

 +and memory tests. The former use gnu time, and the latter use
 +valgrind.
 +
  Pre-requisites
  --
  
 @@ -5,9 +12,10 @@ In addition to having notmuch, you need:
  
  - gpg
  - gnu tar
 -- gnu time
 +- gnu time (for the time tests).

No period?

  - xz. Some speedup can be gotten by installing pixz, but this is
probably only worthwhile if you are debugging the tests.
 +- valgrind (for the memory tests)
  
  Getting set up to run tests:
  
 @@ -36,34 +44,47 @@ for a list of mirrors.
  Running tests
  -
  
 -The easiest way to run performance tests is to say make time-test, (or
 -simply run the notmuch-time-test script). Either command will run all
 -available performance tests.
 -
 -Alternately, you can run a specific subset of tests by simply invoking
 -one of the executable scripts in this directory, (such as ./basic).
 -Each test script supports the following arguments
 +The easiest way to run performance tests is to say make perf-test.
 +This will run all time and memory tests.  Be aware that the memory
 +tests are quite time consuming when run on the full corpus, and that
 +depending on your interests it may be more sensible to run make
 +time-test or make memory-test.  You can also invoke one of the
 +scripts 

proposed fix for memory leak in notmuch restore

2012-12-16 Thread david
In id:87vcc2q5n2@nikula.org, Jani points out a memory leak in the
new restore code.  This series is a proposed fix, namely do any
allocation on a temporary talloc context that is destroyed after each
line is processed.


___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 4b76d83..52e7ecb 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;
 
@@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;
 
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, query_string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  query_string, tag_ops);
 
if (ret == 0) {
@@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
break;
}
 
+   talloc_free (line_ctx);
+   /* setting to NULL is important to avoid a double free */
+   line_ctx = NULL;
 }  while ((line_len = getline (line, line_size, input)) != -1);
 
+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree (regex);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

This code is no less correct than the previous version, since it does
not make sense for the array to live longer than the wrapping struct.

By not relying on the context passed into tag_parse_line, we can allow
tag_op_list_t structures to live longer than that context.
---
 notmuch-restore.c |2 +-
 tag-util.c|9 -
 tag-util.h|3 +--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..ae0ef45 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -102,7 +102,7 @@ parse_sup_line (void *ctx, char *line,
tok_len++;
}
 
-   if (tag_op_list_append (ctx, tag_ops, tok, FALSE))
+   if (tag_op_list_append (tag_ops, tok, FALSE))
return -1;
 }
 
diff --git a/tag-util.c b/tag-util.c
index eab482f..705b7ba 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line,
goto DONE;
}
 
-   if (tag_op_list_append (ctx, tag_ops, tag, remove)) {
+   if (tag_op_list_append (tag_ops, tag, remove)) {
ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error,
  aborting);
goto DONE;
@@ -294,7 +294,7 @@ tag_op_list_create (void *ctx)
 list-size = TAG_OP_LIST_INITIAL_SIZE;
 list-count = 0;
 
-list-ops = talloc_array (ctx, tag_operation_t, list-size);
+list-ops = talloc_array (list, tag_operation_t, list-size);
 if (list-ops == NULL)
return NULL;
 
@@ -303,8 +303,7 @@ tag_op_list_create (void *ctx)
 
 
 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove)
 {
@@ -314,7 +313,7 @@ tag_op_list_append (void *ctx,
 
 if (list-count == list-size) {
list-size *= 2;
-   list-ops = talloc_realloc (ctx, list-ops, tag_operation_t,
+   list-ops = talloc_realloc (list, list-ops, tag_operation_t,
list-size);
if (list-ops == NULL) {
fprintf (stderr, Out of memory.\n);
diff --git a/tag-util.h b/tag-util.h
index 99b0fa0..c07bfde 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -87,8 +87,7 @@ tag_op_list_create (void *ctx);
  */
 
 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [RFC PATCH] cli: add --remove-all option to notmuch tag

2012-12-16 Thread Mark Walters

On Mon, 03 Dec 2012, Jani Nikula j...@nikula.org wrote:
 Add --remove-all option to notmuch tag to remove all tags matching
 query before applying the tag changes. This allows removal and
 unconditional setting of the tags of a message:

 $ notmuch tag --remove-all id:f...@example.com
 $ notmuch tag --remove-all +foo +bar id:f...@example.com

 without having to resort to the complicated (and still quoting broken):

 $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') 
 id:f...@example.com
 $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar 
 id:f...@example.com

 ---

 It's completely untested...

 This is on top of David's batch tagging branch new-tagging at
 git://pivot.cs.unb.ca/notmuch.git

 If there's interest, I'll spin a new version with tests and man page
 changes after David's stuff has been merged.

I like this: the possibility of setting the tags to something seems
nice. 

I am not very keen on the option name but don't have anything much
better: maybe --remove-all-first or --set-to? 

Incidentally, does this (or should this) give an error if the user calls
--remove-all with -some_tag (as opposed to +some_tag)?

Best wishes

Mark


 ---
  notmuch-tag.c |   32 
  1 file changed, 20 insertions(+), 12 deletions(-)

 diff --git a/notmuch-tag.c b/notmuch-tag.c
 index e4fca67..67624cc 100644
 --- a/notmuch-tag.c
 +++ b/notmuch-tag.c
 @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
 const char *query_string,
  notmuch_messages_t *messages;
  notmuch_message_t *message;
  
 -/* Optimize the query so it excludes messages that already have
 - * the specified set of tags. */
 -query_string = _optimize_tag_query (ctx, query_string, tag_ops);
 -if (query_string == NULL) {
 - fprintf (stderr, Out of memory.\n);
 - return 1;
 +if (! (flags  TAG_FLAG_REMOVE_ALL)) {
 + /* Optimize the query so it excludes messages that already
 +  * have the specified set of tags. */
 + query_string = _optimize_tag_query (ctx, query_string, tag_ops);
 + if (query_string == NULL) {
 + fprintf (stderr, Out of memory.\n);
 + return 1;
 + }
 + flags |= TAG_FLAG_PRE_OPTIMIZED;
  }
  
  query = notmuch_query_create (notmuch, query_string);
 @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
 char *query_string,
notmuch_messages_valid (messages)  ! interrupted;
notmuch_messages_move_to_next (messages)) {
   message = notmuch_messages_get (messages);
 - tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
 + tag_op_list_apply (message, tag_ops, flags);
   notmuch_message_destroy (message);
  }
  
 @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  notmuch_config_t *config;
  notmuch_database_t *notmuch;
  struct sigaction action;
 -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
 +tag_op_flag_t flags = TAG_FLAG_NONE;
  notmuch_bool_t batch = FALSE;
 +notmuch_bool_t remove_all = FALSE;
  FILE *input = stdin;
  char *input_file_name = NULL;
  int i, opt_index;
 @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  notmuch_opt_desc_t options[] = {
   { NOTMUCH_OPT_BOOLEAN, batch, batch, 0, 0 },
   { NOTMUCH_OPT_STRING, input_file_name, input, 'i', 0 },
 + { NOTMUCH_OPT_BOOLEAN, remove_all, remove-all, 0, 0 },
   { 0, 0, 0, 0, 0 }
  };
  
 @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
   }
   }
  
 - if (tag_op_list_size (tag_ops) == 0) {
 + if (tag_op_list_size (tag_ops) == 0  !remove_all) {
   fprintf (stderr, Error: 'notmuch tag' requires at least one tag to 
 add or remove.\n);
   return 1;
   }
 @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
   return 1;
  
  if (notmuch_config_get_maildir_synchronize_flags (config))
 - synchronize_flags = TAG_FLAG_MAILDIR_SYNC;
 + flags |= TAG_FLAG_MAILDIR_SYNC;
 +
 +if (remove_all)
 + flags |= TAG_FLAG_REMOVE_ALL;
  
  if (batch) {
  
 @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
   continue;
  
   if (ret  0 || tag_query (ctx, notmuch, query_string,
 -   tag_ops, synchronize_flags))
 +   tag_ops, flags))
   break;
   }
  
   if (line)
   free (line);
  } else
 - ret = tag_query (ctx, notmuch, query_string, tag_ops, 
 synchronize_flags);
 + ret = tag_query (ctx, notmuch, query_string, tag_ops, flags);
  
  notmuch_database_destroy (notmuch);
  
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch

Re: gmail importer script

2012-12-16 Thread Jason A. Donenfeld
On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke patricktot...@gmail.comwrote:

 Well, thats not the point.. the script shouldn't die like this.
 I think it's be better if the script caught that exception, deleted the
 file
 and continued..


Probably, but I suspect it's related to whatever mystery error you ran into
before with the corruption.

Does deleting that file and trying again fix it?


Anyway, this is extremely stable for me and a few others at this point. I'm
going to wait for other users to report errors. Alternatively, send me
patches if you want things to happen.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Jani Nikula
Hi all, v3 of id:cover.1355064714.git.j...@nikula.org

Changes since v2:
 * add new patch 1/5 to clarify sprinter documentation
 * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and
   id:87y5gyvvv7@awakening.csail.mit.edu

Diff to v2 at the end of the cover letter.


BR,
Jani.


Jani Nikula (5):
  sprinter: clarify separator documentation
  sprinter: add text0 formatter for null character separated text
  cli: add --format=text0 to notmuch search
  test: notmuch search --format=text0
  man: document notmuch search --format=text0

 man/man1/notmuch-search.1 |   26 --
 notmuch-search.c  |   16 ++--
 sprinter-text.c   |   22 ++
 sprinter.h|   15 +++
 test/text |   33 +
 5 files changed, 96 insertions(+), 16 deletions(-)

-- 
1.7.10.4



diff --git a/sprinter.h b/sprinter.h
index f36b999..f859672 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -42,10 +42,11 @@ typedef struct sprinter {
  */
 void (*map_key) (struct sprinter *, const char *);
 
-/* Insert a separator (usually extra whitespace) for improved
- * readability without affecting the abstract syntax of the
- * structure being printed.
- * For JSON, this could simply be a line break.
+/* Insert a separator (usually extra whitespace). For the text
+ * printers, this is a syntactic separator. For the structured
+ * printers, this is for improved readability without affecting
+ * the abstract syntax of the structure being printed. For JSON,
+ * this could simply be a line break.
  */
 void (*separator) (struct sprinter *);
 
diff --git a/test/text b/test/text
index e003a66..b5ccefc 100755
--- a/test/text
+++ b/test/text
@@ -55,30 +55,34 @@ test_expect_equal $output \
 add_email_corpus
 
 test_begin_subtest Search message tags: text0
-cat EOF  EXPECTED.$test_count
+cat EOF  EXPECTED
 attachment inbox signed unread
 EOF
-notmuch search --format=text0 --output=tags '*' | xargs -0 | 
notmuch_search_sanitize  OUTPUT.$test_count
-test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+notmuch search --format=text0 --output=tags '*' | xargs -0 | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
 
+# Use tr(1) to convert --output=text0 to --output=text for
+# comparison. Also translate newlines to spaces to fail with more
+# noise if they are present as delimiters instead of null
+# characters. This assumes there are no newlines in the data.
 test_begin_subtest Compare text vs. text0 for threads
-notmuch search --format=text --output=threads '*' | notmuch_search_sanitize  
EXPECTED.$test_count
-notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
notmuch_search_sanitize  OUTPUT.$test_count
-test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+notmuch search --format=text --output=threads '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=threads '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest Compare text vs. text0 for messages
-notmuch search --format=text --output=messages '*' | notmuch_search_sanitize  
EXPECTED.$test_count
-notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
notmuch_search_sanitize  OUTPUT.$test_count
-test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+notmuch search --format=text --output=messages '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=messages '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest Compare text vs. text0 for files
-notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
EXPECTED.$test_count
-notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
notmuch_search_sanitize  OUTPUT.$test_count
-test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=files '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest Compare text vs. text0 for tags
-notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
EXPECTED.$test_count
-notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
notmuch_search_sanitize  OUTPUT.$test_count
-test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=tags '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
 
 test_done
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 1/5] sprinter: clarify separator documentation

2012-12-16 Thread Jani Nikula
For text printers, the separator is a syntactic element.
---
 sprinter.h |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sprinter.h b/sprinter.h
index 59776a9..f43a844 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -42,10 +42,11 @@ typedef struct sprinter {
  */
 void (*map_key) (struct sprinter *, const char *);
 
-/* Insert a separator (usually extra whitespace) for improved
- * readability without affecting the abstract syntax of the
- * structure being printed.
- * For JSON, this could simply be a line break.
+/* Insert a separator (usually extra whitespace). For the text
+ * printers, this is a syntactic separator. For the structured
+ * printers, this is for improved readability without affecting
+ * the abstract syntax of the structure being printed. For JSON,
+ * this could simply be a line break.
  */
 void (*separator) (struct sprinter *);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 2/5] sprinter: add text0 formatter for null character separated text

2012-12-16 Thread Jani Nikula
Same as the text formatter, but with each field separated by a null
character rather than a newline character.
---
 sprinter-text.c |   22 ++
 sprinter.h  |6 ++
 2 files changed, 28 insertions(+)

diff --git a/sprinter-text.c b/sprinter-text.c
index 10343be..7779488 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -68,6 +68,14 @@ text_separator (struct sprinter *sp)
 }
 
 static void
+text0_separator (struct sprinter *sp)
+{
+struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+fputc ('\0', sptxt-stream);
+}
+
+static void
 text_set_prefix (struct sprinter *sp, const char *prefix)
 {
 struct sprinter_text *sptxt = (struct sprinter_text *) sp;
@@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream)
 res-stream = stream;
 return res-vtable;
 }
+
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream)
+{
+struct sprinter *sp;
+
+sp = sprinter_text_create (ctx, stream);
+if (! sp)
+   return NULL;
+
+sp-separator = text0_separator;
+
+return sp;
+}
diff --git a/sprinter.h b/sprinter.h
index f43a844..f859672 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -67,6 +67,12 @@ typedef struct sprinter {
 struct sprinter *
 sprinter_text_create (const void *ctx, FILE *stream);
 
+/* Create a new unstructured printer that emits the text format for
+ * notmuch search, with each field separated by a null character
+ * instead of the newline character. */
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream);
+
 /* Create a new structure printer that emits JSON. */
 struct sprinter *
 sprinter_json_create (const void *ctx, FILE *stream);
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 3/5] cli: add --format=text0 to notmuch search

2012-12-16 Thread Jani Nikula
Add new format text0, which is otherwise the same as text, but use the
null character as separator instead of the newline character. This is
similar to find(1) -print0 option, and works together with the
xargs(1) -0 option.
---
 notmuch-search.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 6218622..627962b 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 int exclude = EXCLUDE_TRUE;
 unsigned int i;
 
-enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
-   format_sel = NOTMUCH_FORMAT_TEXT;
+enum {
+   NOTMUCH_FORMAT_JSON,
+   NOTMUCH_FORMAT_TEXT,
+   NOTMUCH_FORMAT_TEXT0,
+   NOTMUCH_FORMAT_SEXP
+} format_sel = NOTMUCH_FORMAT_TEXT;
 
 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_KEYWORD, sort, sort, 's',
@@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
  (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
  { sexp, NOTMUCH_FORMAT_SEXP },
  { text, NOTMUCH_FORMAT_TEXT },
+ { text0, NOTMUCH_FORMAT_TEXT0 },
  { 0, 0 } } },
{ NOTMUCH_OPT_KEYWORD, output, output, 'o',
  (notmuch_keyword_t []){ { summary, OUTPUT_SUMMARY },
@@ -345,6 +350,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 case NOTMUCH_FORMAT_TEXT:
format = sprinter_text_create (ctx, stdout);
break;
+case NOTMUCH_FORMAT_TEXT0:
+   if (output == OUTPUT_SUMMARY) {
+   fprintf (stderr, Error: --format=text0 is not compatible with 
--output=summary.\n);
+   return 1;
+   }
+   format = sprinter_text0_create (ctx, stdout);
+   break;
 case NOTMUCH_FORMAT_JSON:
format = sprinter_json_create (ctx, stdout);
break;
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 4/5] test: notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 test/text |   33 +
 1 file changed, 33 insertions(+)

diff --git a/test/text b/test/text
index 428c89b..b5ccefc 100755
--- a/test/text
+++ b/test/text
@@ -52,4 +52,37 @@ output=$(notmuch search --format=text tëxt-search-méssage 
| notmuch_search_s
 test_expect_equal $output \
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
text-search-utf8-body-sübjéct (inbox unread)
 
+add_email_corpus
+
+test_begin_subtest Search message tags: text0
+cat EOF  EXPECTED
+attachment inbox signed unread
+EOF
+notmuch search --format=text0 --output=tags '*' | xargs -0 | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+# Use tr(1) to convert --output=text0 to --output=text for
+# comparison. Also translate newlines to spaces to fail with more
+# noise if they are present as delimiters instead of null
+# characters. This assumes there are no newlines in the data.
+test_begin_subtest Compare text vs. text0 for threads
+notmuch search --format=text --output=threads '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=threads '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for messages
+notmuch search --format=text --output=messages '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=messages '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for files
+notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=files '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for tags
+notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=tags '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 5/5] man: document notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 man/man1/notmuch-search.1 |   26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 0aff348..22bcd0a 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -25,9 +25,11 @@ Supported options for
 include
 .RS 4
 .TP 4
-.BR \-\-format= ( json | sexp | text )
+.BR \-\-format= ( json | sexp | text | text0 )
 
-Presents the results in either JSON, S-Expressions or plain-text (default).
+Presents the results in either JSON, S-Expressions, newline character
+separated plain-text (default), or null character separated plain-text
+(compatible with \fBxargs\fR(1) -0 option where available).
 .RE
 
 .RS 4
@@ -48,32 +50,36 @@ the authors of the thread and the subject.
 .B threads
 
 Output the thread IDs of all threads with any message matching the
-search terms, either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or an S-Expression list (\-\-format=sexp).
+search terms, either one per line (\-\-format=text), separated by null
+characters (\-\-format=text0), as a JSON array (\-\-format=json), or
+an S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B messages
 
 Output the message IDs of all messages matching the search terms,
-either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B files
 
 Output the filenames of all messages matching the search terms, either
-one per line (\-\-format=text) or as a JSON array (\-\-format=json) or
-as an S-Expression list (\-\-format=sexp).
+one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B tags
 
 Output all tags that appear on any message matching the search terms,
-either one per line (\-\-format=text) or as a JSON array (\-\-format=json)
-or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RE
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Austin Clements
On Sun, 16 Dec 2012, Jani Nikula j...@nikula.org wrote:
 Hi all, v3 of id:cover.1355064714.git.j...@nikula.org

 Changes since v2:
  * add new patch 1/5 to clarify sprinter documentation
  * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and
id:87y5gyvvv7@awakening.csail.mit.edu

 Diff to v2 at the end of the cover letter.


 BR,
 Jani.

LGTM.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v3 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Mark Walters
On Sun, 16 Dec 2012, Jani Nikula j...@nikula.org wrote:
 Hi all, v3 of id:cover.1355064714.git.j...@nikula.org

 Changes since v2:
  * add new patch 1/5 to clarify sprinter documentation
  * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and
id:87y5gyvvv7@awakening.csail.mit.edu

 Diff to v2 at the end of the cover letter.

LGTM +1

Best wishes

Mark





 BR,
 Jani.


 Jani Nikula (5):
   sprinter: clarify separator documentation
   sprinter: add text0 formatter for null character separated text
   cli: add --format=text0 to notmuch search
   test: notmuch search --format=text0
   man: document notmuch search --format=text0

  man/man1/notmuch-search.1 |   26 --
  notmuch-search.c  |   16 ++--
  sprinter-text.c   |   22 ++
  sprinter.h|   15 +++
  test/text |   33 +
  5 files changed, 96 insertions(+), 16 deletions(-)

 -- 
 1.7.10.4



 diff --git a/sprinter.h b/sprinter.h
 index f36b999..f859672 100644
 --- a/sprinter.h
 +++ b/sprinter.h
 @@ -42,10 +42,11 @@ typedef struct sprinter {
   */
  void (*map_key) (struct sprinter *, const char *);
  
 -/* Insert a separator (usually extra whitespace) for improved
 - * readability without affecting the abstract syntax of the
 - * structure being printed.
 - * For JSON, this could simply be a line break.
 +/* Insert a separator (usually extra whitespace). For the text
 + * printers, this is a syntactic separator. For the structured
 + * printers, this is for improved readability without affecting
 + * the abstract syntax of the structure being printed. For JSON,
 + * this could simply be a line break.
   */
  void (*separator) (struct sprinter *);
  
 diff --git a/test/text b/test/text
 index e003a66..b5ccefc 100755
 --- a/test/text
 +++ b/test/text
 @@ -55,30 +55,34 @@ test_expect_equal $output \
  add_email_corpus
  
  test_begin_subtest Search message tags: text0
 -cat EOF  EXPECTED.$test_count
 +cat EOF  EXPECTED
  attachment inbox signed unread
  EOF
 -notmuch search --format=text0 --output=tags '*' | xargs -0 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +notmuch search --format=text0 --output=tags '*' | xargs -0 | 
 notmuch_search_sanitize  OUTPUT
 +test_expect_equal_file EXPECTED OUTPUT
  
 +# Use tr(1) to convert --output=text0 to --output=text for
 +# comparison. Also translate newlines to spaces to fail with more
 +# noise if they are present as delimiters instead of null
 +# characters. This assumes there are no newlines in the data.
  test_begin_subtest Compare text vs. text0 for threads
 -notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
  EXPECTED.$test_count
 -notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize 
  EXPECTED
 +notmuch search --format=text0 --output=threads '*' | tr \n\0  \n | 
 notmuch_search_sanitize  OUTPUT
 +test_expect_equal_file EXPECTED OUTPUT
  
  test_begin_subtest Compare text vs. text0 for messages
 -notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
  EXPECTED.$test_count
 -notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize 
  EXPECTED
 +notmuch search --format=text0 --output=messages '*' | tr \n\0  \n | 
 notmuch_search_sanitize  OUTPUT
 +test_expect_equal_file EXPECTED OUTPUT
  
  test_begin_subtest Compare text vs. text0 for files
 -notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
 EXPECTED.$test_count
 -notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
 EXPECTED
 +notmuch search --format=text0 --output=files '*' | tr \n\0  \n | 
 notmuch_search_sanitize  OUTPUT
 +test_expect_equal_file EXPECTED OUTPUT
  
  test_begin_subtest Compare text vs. text0 for tags
 -notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
 EXPECTED.$test_count
 -notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | 
 notmuch_search_sanitize  OUTPUT.$test_count
 -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
 EXPECTED
 +notmuch search --format=text0 --output=tags '*' | tr \n\0  \n | 
 notmuch_search_sanitize  OUTPUT
 +test_expect_equal_file 

Re: [PATCH v2 1/7] emacs: Centralize notmuch command error handling

2012-12-16 Thread David Bremner
Austin Clements amdra...@mit.edu writes:

 This provides library functions for unified handling of errors from
 the notmuch CLI.  Follow-up patches will convert some scattered error
 handling to use this and add error handling where we currently ignore
 errors.

pushed, 

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [RFC PATCH] cli: add --remove-all option to notmuch tag

2012-12-16 Thread Jani Nikula
On Sun, 16 Dec 2012, Mark Walters markwalters1...@gmail.com wrote:
 On Mon, 03 Dec 2012, Jani Nikula j...@nikula.org wrote:
 Add --remove-all option to notmuch tag to remove all tags matching
 query before applying the tag changes. This allows removal and
 unconditional setting of the tags of a message:

 $ notmuch tag --remove-all id:f...@example.com
 $ notmuch tag --remove-all +foo +bar id:f...@example.com

 without having to resort to the complicated (and still quoting broken):

 $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') 
 id:f...@example.com
 $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar 
 id:f...@example.com

 ---

 It's completely untested...

 This is on top of David's batch tagging branch new-tagging at
 git://pivot.cs.unb.ca/notmuch.git

 If there's interest, I'll spin a new version with tests and man page
 changes after David's stuff has been merged.

 I like this: the possibility of setting the tags to something seems
 nice. 

Thanks for the feedback; I'll rebase once we're done with the batch
tagging stuff.

 I am not very keen on the option name but don't have anything much
 better: maybe --remove-all-first or --set-to? 

I still like --remove-all, and, like you say, those aren't much
better. But we can bikeshed this later. ;)

 Incidentally, does this (or should this) give an error if the user calls
 --remove-all with -some_tag (as opposed to +some_tag)?

I think not. There are no errors if you do -foo -foo, or remove a tag
that isn't in any of the messages matching query.

BR,
Jani.



 Best wishes

 Mark


 ---
  notmuch-tag.c |   32 
  1 file changed, 20 insertions(+), 12 deletions(-)

 diff --git a/notmuch-tag.c b/notmuch-tag.c
 index e4fca67..67624cc 100644
 --- a/notmuch-tag.c
 +++ b/notmuch-tag.c
 @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
 const char *query_string,
  notmuch_messages_t *messages;
  notmuch_message_t *message;
  
 -/* Optimize the query so it excludes messages that already have
 - * the specified set of tags. */
 -query_string = _optimize_tag_query (ctx, query_string, tag_ops);
 -if (query_string == NULL) {
 -fprintf (stderr, Out of memory.\n);
 -return 1;
 +if (! (flags  TAG_FLAG_REMOVE_ALL)) {
 +/* Optimize the query so it excludes messages that already
 + * have the specified set of tags. */
 +query_string = _optimize_tag_query (ctx, query_string, tag_ops);
 +if (query_string == NULL) {
 +fprintf (stderr, Out of memory.\n);
 +return 1;
 +}
 +flags |= TAG_FLAG_PRE_OPTIMIZED;
  }
  
  query = notmuch_query_create (notmuch, query_string);
 @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
 char *query_string,
   notmuch_messages_valid (messages)  ! interrupted;
   notmuch_messages_move_to_next (messages)) {
  message = notmuch_messages_get (messages);
 -tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED);
 +tag_op_list_apply (message, tag_ops, flags);
  notmuch_message_destroy (message);
  }
  
 @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  notmuch_config_t *config;
  notmuch_database_t *notmuch;
  struct sigaction action;
 -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE;
 +tag_op_flag_t flags = TAG_FLAG_NONE;
  notmuch_bool_t batch = FALSE;
 +notmuch_bool_t remove_all = FALSE;
  FILE *input = stdin;
  char *input_file_name = NULL;
  int i, opt_index;
 @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  notmuch_opt_desc_t options[] = {
  { NOTMUCH_OPT_BOOLEAN, batch, batch, 0, 0 },
  { NOTMUCH_OPT_STRING, input_file_name, input, 'i', 0 },
 +{ NOTMUCH_OPT_BOOLEAN, remove_all, remove-all, 0, 0 },
  { 0, 0, 0, 0, 0 }
  };
  
 @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  }
  }
  
 -if (tag_op_list_size (tag_ops) == 0) {
 +if (tag_op_list_size (tag_ops) == 0  !remove_all) {
  fprintf (stderr, Error: 'notmuch tag' requires at least one tag to 
 add or remove.\n);
  return 1;
  }
 @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  return 1;
  
  if (notmuch_config_get_maildir_synchronize_flags (config))
 -synchronize_flags = TAG_FLAG_MAILDIR_SYNC;
 +flags |= TAG_FLAG_MAILDIR_SYNC;
 +
 +if (remove_all)
 +flags |= TAG_FLAG_REMOVE_ALL;
  
  if (batch) {
  
 @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
  continue;
  
  if (ret  0 || tag_query (ctx, notmuch, query_string,
 -  tag_ops, synchronize_flags))
 +  tag_ops, flags))
  break;
  }
  
  if (line)
  free (line);
  } else
 -ret = tag_query (ctx, notmuch, 

[PATCH v4 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Jani Nikula
Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix
a tiny conflict in patch context of 3/5.

BR,
Jani.


Jani Nikula (5):
  sprinter: clarify separator documentation
  sprinter: add text0 formatter for null character separated text
  cli: add --format=text0 to notmuch search
  test: notmuch search --format=text0
  man: document notmuch search --format=text0

 man/man1/notmuch-search.1 |   26 --
 notmuch-search.c  |   16 ++--
 sprinter-text.c   |   22 ++
 sprinter.h|   15 +++
 test/text |   33 +
 5 files changed, 96 insertions(+), 16 deletions(-)

-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 1/5] sprinter: clarify separator documentation

2012-12-16 Thread Jani Nikula
For text printers, the separator is a syntactic element.
---
 sprinter.h |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sprinter.h b/sprinter.h
index 59776a9..f43a844 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -42,10 +42,11 @@ typedef struct sprinter {
  */
 void (*map_key) (struct sprinter *, const char *);
 
-/* Insert a separator (usually extra whitespace) for improved
- * readability without affecting the abstract syntax of the
- * structure being printed.
- * For JSON, this could simply be a line break.
+/* Insert a separator (usually extra whitespace). For the text
+ * printers, this is a syntactic separator. For the structured
+ * printers, this is for improved readability without affecting
+ * the abstract syntax of the structure being printed. For JSON,
+ * this could simply be a line break.
  */
 void (*separator) (struct sprinter *);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 2/5] sprinter: add text0 formatter for null character separated text

2012-12-16 Thread Jani Nikula
Same as the text formatter, but with each field separated by a null
character rather than a newline character.
---
 sprinter-text.c |   22 ++
 sprinter.h  |6 ++
 2 files changed, 28 insertions(+)

diff --git a/sprinter-text.c b/sprinter-text.c
index 10343be..7779488 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -68,6 +68,14 @@ text_separator (struct sprinter *sp)
 }
 
 static void
+text0_separator (struct sprinter *sp)
+{
+struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+fputc ('\0', sptxt-stream);
+}
+
+static void
 text_set_prefix (struct sprinter *sp, const char *prefix)
 {
 struct sprinter_text *sptxt = (struct sprinter_text *) sp;
@@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream)
 res-stream = stream;
 return res-vtable;
 }
+
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream)
+{
+struct sprinter *sp;
+
+sp = sprinter_text_create (ctx, stream);
+if (! sp)
+   return NULL;
+
+sp-separator = text0_separator;
+
+return sp;
+}
diff --git a/sprinter.h b/sprinter.h
index f43a844..f859672 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -67,6 +67,12 @@ typedef struct sprinter {
 struct sprinter *
 sprinter_text_create (const void *ctx, FILE *stream);
 
+/* Create a new unstructured printer that emits the text format for
+ * notmuch search, with each field separated by a null character
+ * instead of the newline character. */
+struct sprinter *
+sprinter_text0_create (const void *ctx, FILE *stream);
+
 /* Create a new structure printer that emits JSON. */
 struct sprinter *
 sprinter_json_create (const void *ctx, FILE *stream);
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 3/5] cli: add --format=text0 to notmuch search

2012-12-16 Thread Jani Nikula
Add new format text0, which is otherwise the same as text, but use the
null character as separator instead of the newline character. This is
similar to find(1) -print0 option, and works together with the
xargs(1) -0 option.
---
 notmuch-search.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 7704915..0b0a879 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 int exclude = EXCLUDE_TRUE;
 unsigned int i;
 
-enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
-   format_sel = NOTMUCH_FORMAT_TEXT;
+enum {
+   NOTMUCH_FORMAT_JSON,
+   NOTMUCH_FORMAT_TEXT,
+   NOTMUCH_FORMAT_TEXT0,
+   NOTMUCH_FORMAT_SEXP
+} format_sel = NOTMUCH_FORMAT_TEXT;
 
 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_KEYWORD, sort, sort, 's',
@@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
  (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
  { sexp, NOTMUCH_FORMAT_SEXP },
  { text, NOTMUCH_FORMAT_TEXT },
+ { text0, NOTMUCH_FORMAT_TEXT0 },
  { 0, 0 } } },
{ NOTMUCH_OPT_INT, notmuch_format_version, format-version, 0, 0 },
{ NOTMUCH_OPT_KEYWORD, output, output, 'o',
@@ -346,6 +351,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 case NOTMUCH_FORMAT_TEXT:
format = sprinter_text_create (ctx, stdout);
break;
+case NOTMUCH_FORMAT_TEXT0:
+   if (output == OUTPUT_SUMMARY) {
+   fprintf (stderr, Error: --format=text0 is not compatible with 
--output=summary.\n);
+   return 1;
+   }
+   format = sprinter_text0_create (ctx, stdout);
+   break;
 case NOTMUCH_FORMAT_JSON:
format = sprinter_json_create (ctx, stdout);
break;
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 4/5] test: notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 test/text |   33 +
 1 file changed, 33 insertions(+)

diff --git a/test/text b/test/text
index 428c89b..b5ccefc 100755
--- a/test/text
+++ b/test/text
@@ -52,4 +52,37 @@ output=$(notmuch search --format=text tëxt-search-méssage 
| notmuch_search_s
 test_expect_equal $output \
 thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; 
text-search-utf8-body-sübjéct (inbox unread)
 
+add_email_corpus
+
+test_begin_subtest Search message tags: text0
+cat EOF  EXPECTED
+attachment inbox signed unread
+EOF
+notmuch search --format=text0 --output=tags '*' | xargs -0 | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+# Use tr(1) to convert --output=text0 to --output=text for
+# comparison. Also translate newlines to spaces to fail with more
+# noise if they are present as delimiters instead of null
+# characters. This assumes there are no newlines in the data.
+test_begin_subtest Compare text vs. text0 for threads
+notmuch search --format=text --output=threads '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=threads '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for messages
+notmuch search --format=text --output=messages '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=messages '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for files
+notmuch search --format=text --output=files '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=files '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest Compare text vs. text0 for tags
+notmuch search --format=text --output=tags '*' | notmuch_search_sanitize  
EXPECTED
+notmuch search --format=text0 --output=tags '*' | tr \n\0  \n | 
notmuch_search_sanitize  OUTPUT
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v4 5/5] man: document notmuch search --format=text0

2012-12-16 Thread Jani Nikula
---
 man/man1/notmuch-search.1 |   26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 5c771fa..12f6719 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -25,9 +25,11 @@ Supported options for
 include
 .RS 4
 .TP 4
-.BR \-\-format= ( json | sexp | text )
+.BR \-\-format= ( json | sexp | text | text0 )
 
-Presents the results in either JSON, S-Expressions or plain-text (default).
+Presents the results in either JSON, S-Expressions, newline character
+separated plain-text (default), or null character separated plain-text
+(compatible with \fBxargs\fR(1) -0 option where available).
 .RE
 
 .RS 4
@@ -57,32 +59,36 @@ the authors of the thread and the subject.
 .B threads
 
 Output the thread IDs of all threads with any message matching the
-search terms, either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or an S-Expression list (\-\-format=sexp).
+search terms, either one per line (\-\-format=text), separated by null
+characters (\-\-format=text0), as a JSON array (\-\-format=json), or
+an S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B messages
 
 Output the message IDs of all messages matching the search terms,
-either one per line (\-\-format=text) or as a JSON array
-(\-\-format=json) or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B files
 
 Output the filenames of all messages matching the search terms, either
-one per line (\-\-format=text) or as a JSON array (\-\-format=json) or
-as an S-Expression list (\-\-format=sexp).
+one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RS 4
 .TP 4
 .B tags
 
 Output all tags that appear on any message matching the search terms,
-either one per line (\-\-format=text) or as a JSON array (\-\-format=json)
-or as an S-Expression list (\-\-format=sexp).
+either one per line (\-\-format=text), separated by null characters
+(\-\-format=text0), as a JSON array (\-\-format=json), or as an
+S-Expression list (\-\-format=sexp).
 .RE
 .RE
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 4/4] perf-test: add memory leak test for dump restore

2012-12-16 Thread David Bremner
da...@tethera.net writes:
 +
 +memory_run 'load nmbug tags' 'notmuch restore --accumulate 
 --input=corpus.tags/nmbug.sup-dump'
 +memory_run 'dump *' 'notmuch dump --output=tags.sup'
 +memory_run 'restore *' 'notmuch restore --input=tags.sup'
 +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag 
 --output=tags.bt'
 +memory_run 'restore --format=batch-tag *' 'notmuch restore 
 --format=batch-tag --input=tags.bt'
 +

We were talking on IRC about how/if valgrind would cope with talloc, and
the possibility that chunks of memory are still reachable by talloc, but
not by user code.  Currently the talloc context local in main() is
(slightly perversely) only freed in the case of return 1, so all the
memory allocated by talloc on that contex is shown as leaked:

 3,005,500 (93 direct, 3,005,407 indirect) bytes in 1 blocks are definitely 
lost in loss record 553 of 553
at 0x4C2A26B: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x55F14C7: talloc_strndup (in 
/usr/lib/x86_64-linux-gnu/libtalloc.so.2.0.7)
by 0x4115E8: parse_sup_line (notmuch-restore.c:90)
by 0x411AD4: notmuch_restore_command (notmuch-restore.c:209)
by 0x40B2A4: main (notmuch.c:294)

Although this is probably a bug in main(), it does point valgrind to the
right culprit.

As our memory allocation is (alas) a mix of talloc, malloc, and
g_malloc, we probably need both valgrind tests, and some way to toggle
talloc memory debugging.  (
http://talloc.samba.org/talloc/doc/html/group__talloc__debug.html )

d


___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: gmail importer script

2012-12-16 Thread Patrick Totzke
Quoting Jason A. Donenfeld (2012-12-16 20:44:04)
 On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke patricktot...@gmail.com
 wrote:
 
 Well, thats not the point.. the script shouldn't die like this.
 I think it's be better if the script caught that exception, deleted the
 file
 and continued..
 
 
 Probably, but I suspect it's related to whatever mystery error you ran into
 before with the corruption.
 
 Does deleting that file and trying again fix it? 

I don't know. How would I know which file to remove?
I just removed the newest files in new/cur/tmp
but this doesnt solve the issue.
What would really help here is some more informative error message!

I tried with the `-d` option but did not see any local path in the output.

 
 Anyway, this is extremely stable for me and a few others at this point. I'm
 going to wait for other users to report errors. Alternatively, send me patches
 if you want things to happen.

Personally, I don't really care if 'things are happening', as I'm kind of OK 
with my current
offlineimap configuration and would seriously consider switching from OI
only if it meant an improvement in speed or stability.

I test this code because I believe that the notmuch ecosystem
would benefit from a working solution and that reviews and bug-reports
are important.

Of course, I don't expect any miracles in terms of speed at this early stage.
But the bug I reported above should be addressed at some point in order not to
scare off potential users.

Have you considered including your code in offlineimap?
I'm asking because OI already solved some of the problems you might face later 
on
if you want to continue working on this.

* multi-threaded downloads
* the ability to read passwords not as plaintext parameter..

best,
/p


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

2012-12-16 Thread Jani Nikula
On Sun, 16 Dec 2012, da...@tethera.net wrote:
 From: David Bremner brem...@debian.org

 This lets the high level code in notmuch restore be ignorant about
 what the lower level code is doing as far as allocating memory.
 ---
  notmuch-restore.c |   12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

 diff --git a/notmuch-restore.c b/notmuch-restore.c
 index 4b76d83..52e7ecb 100644
 --- a/notmuch-restore.c
 +++ b/notmuch-restore.c
 @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
  char *input_file_name = NULL;
  FILE *input = stdin;
  char *line = NULL;
 +void *line_ctx = NULL;
  size_t line_size;
  ssize_t line_len;
  
 @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
  do {
   char *query_string;

This patch only works on top of the batch tagging series, because
there's still one continue statement in the id: prefix check. But you
could make it work for both, *and* keep the slightly more intuitive ret
checking order if you did (yes, the slightly counter-intuitive):

if (line_ctxt != NULL)
  talloc_free (line_ctx);

right here, and...

 + line_ctx = talloc_new (ctx);
   if (input_format == DUMP_FORMAT_SUP) {
 - ret = parse_sup_line (ctx, line, query_string, tag_ops);
 + ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
   } else {
 - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
 + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
 query_string, tag_ops);
  
   if (ret == 0) {
 @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
   break;
   }
  
 + talloc_free (line_ctx);
 + /* setting to NULL is important to avoid a double free */
 + line_ctx = NULL;

...removed the above lines here.

Otherwise I think you'll need a temporary goto until the batch tagging
series. (I'm fine with that too.)

Overall the series LGTM, and I like how this dodges the query_string
alloc/free problem altogether.

BR,
Jani.

  }  while ((line_len = getline (line, line_size, input)) != -1);
  
 +if (line_ctx != NULL)
 + talloc_free (line_ctx);
 +
  if (input_format == DUMP_FORMAT_SUP)
   regfree (regex);
  
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: handling mail sent to a subscribed list

2012-12-16 Thread Brandon Invergo
 If understand correctly, your concern is with the second copy of a
 message with the same message-id not showing up in your inbox? If so, this is
 more or less a feature (although in the case where the duplicated
 message id's are because of malice or stupidity on the sender's part,
 and not duplicated messages, it is also a known bug).  

 On the command line you can try notmuch search --output=files id:foo
 where id:foo is copied via c i in the emacs interface.

 Or maybe I misunderstand your problem completely.

Thanks for the reply. Yes I think you summed it up; I figured that it
was behaving properly. In short it's that I have the email that I sent
sitting wherever it was Fcc'ed and then shortly thereafter I receive
from the email list a message with the same ID. Since the Fcc'ed one was
already added to the database, the one that I received from the list
just sits there. But I guess others using mailing lists have encountered
this too, so I was asking how they handle it since I haven't figured it
out yet.

I'll give that command a try to see if I can put it to good use somehow.

Regards,
-brandon
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v4 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Tomi Ollila
On Mon, Dec 17 2012, Jani Nikula j...@nikula.org wrote:

 Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix
 a tiny conflict in patch context of 3/5.

 BR,
 Jani.

As Mark  Austin (also) gave  +1 to v3 added notmuch::patch (only).

Tomi


 Jani Nikula (5):
   sprinter: clarify separator documentation
   sprinter: add text0 formatter for null character separated text
   cli: add --format=text0 to notmuch search
   test: notmuch search --format=text0
   man: document notmuch search --format=text0

  man/man1/notmuch-search.1 |   26 --
  notmuch-search.c  |   16 ++--
  sprinter-text.c   |   22 ++
  sprinter.h|   15 +++
  test/text |   33 +
  5 files changed, 96 insertions(+), 16 deletions(-)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v4 0/5] add --format=text0 to notmuch search

2012-12-16 Thread Tomi Ollila
On Mon, Dec 17 2012, Tomi Ollila tomi.oll...@iki.fi wrote:

 On Mon, Dec 17 2012, Jani Nikula j...@nikula.org wrote:

 Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix
 a tiny conflict in patch context of 3/5.

 BR,
 Jani.

 As Mark  Austin (also) gave  +1 to v3 added notmuch::patch (only).

Ok, it was like that already, my nmbug tree was out-of-sync
(had to fix it with nmbug checkout; nmbug pull)...

 Tomi

Tomi


 Jani Nikula (5):
   sprinter: clarify separator documentation
   sprinter: add text0 formatter for null character separated text
   cli: add --format=text0 to notmuch search
   test: notmuch search --format=text0
   man: document notmuch search --format=text0

  man/man1/notmuch-search.1 |   26 --
  notmuch-search.c  |   16 ++--
  sprinter-text.c   |   22 ++
  sprinter.h|   15 +++
  test/text |   33 +
  5 files changed, 96 insertions(+), 16 deletions(-)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] CLI: add talloc leak report, controlled by an environment variable.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

The argument handling in notmuch.c seems due for an overhaul, but
until then use an environment variable to specify a location to write
the talloc leak report to.  This is only enabled for the (interesting)
case where some notmuch subcommand is invoked.
---
 notmuch.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 9516dfb..fb49c5a 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -322,8 +322,20 @@ main (int argc, char *argv[])
 for (i = 0; i  ARRAY_SIZE (commands); i++) {
command = commands[i];
 
-   if (strcmp (argv[1], command-name) == 0)
-   return (command-function) (local, argc - 1, argv[1]);
+   if (strcmp (argv[1], command-name) == 0) {
+   int ret;
+   char *talloc_report;
+
+   ret = (command-function) (local, argc - 1, argv[1]);
+
+   talloc_report=getenv (NOTMUCH_TALLOC_REPORT);
+   if (talloc_report  strcmp(talloc_report, ) != 0) {
+   FILE *report = fopen (talloc_report, w);
+   talloc_report_full (NULL, report);
+   }
+
+   return ret;
+   }
 }
 
 fprintf (stderr, Error: Unknown command '%s' (see \notmuch help\)\n,
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] notmuch-restore: use xtalloc version of strndup

2012-12-16 Thread david
From: David Bremner brem...@debian.org

This gives line numbers for better debugging.
---
 notmuch-restore.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..0cc9c9f 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -85,9 +85,10 @@ parse_sup_line (void *ctx, char *line,
return 1;
 }
 
-*query_str = talloc_strndup (ctx, line + match[1].rm_so,
-match[1].rm_eo - match[1].rm_so);
-file_tags = talloc_strndup (ctx, line + match[2].rm_so,
+*query_str = xtalloc_strndup (ctx, line + match[1].rm_so,
+   match[1].rm_eo - match[1].rm_so);
+
+file_tags = xtalloc_strndup (ctx, line + match[2].rm_so,
match[2].rm_eo - match[2].rm_so);
 
 char *tok = file_tags;
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/3] util: add xtalloc.[ch]

2012-12-16 Thread david
From: David Bremner brem...@debian.org

These are intended to be simple wrappers to provide slightly better
debugging information than what talloc currently provides natively.
---
 notmuch-client.h|2 +-
 util/Makefile.local |2 +-
 util/xtalloc.c  |   15 +++
 util/xtalloc.h  |   18 ++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 util/xtalloc.c
 create mode 100644 util/xtalloc.h

diff --git a/notmuch-client.h b/notmuch-client.h
index d7b352e..60be030 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include errno.h
 #include signal.h
 
-#include talloc.h
+#include xtalloc.h
 
 #define unused(x) x __attribute__ ((unused))
 
diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..8a62c00 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -4,7 +4,7 @@ dir := util
 extra_cflags += -I$(srcdir)/$(dir)
 
 libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
- $(dir)/string-util.c
+ $(dir)/string-util.c $(dir)/xtalloc.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/xtalloc.c b/util/xtalloc.c
new file mode 100644
index 000..22834bd
--- /dev/null
+++ b/util/xtalloc.c
@@ -0,0 +1,15 @@
+#include string.h
+#include xtalloc.h
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_named_const (ctx, len + 1, name);
+
+if (ptr) {
+   memcpy (ptr, str, len);
+   *(ptr + len) = '\0';
+}
+return ptr;
+}
diff --git a/util/xtalloc.h b/util/xtalloc.h
new file mode 100644
index 000..3cc1179
--- /dev/null
+++ b/util/xtalloc.h
@@ -0,0 +1,18 @@
+#ifndef _XTALLOC_H
+#define _XTALLOC_H
+
+#include talloc.h
+
+/* Like talloc_strndup, but take an extra parameter for the internal talloc
+ * name (for debugging) */
+
+char *
+xtalloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name);
+
+/* use the __location__ macro from talloc.h to name a string according to its
+ * source location */
+
+#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, 
len, __location__)
+
+#endif
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


v2 restore leak fix

2012-12-16 Thread david
This obsoletes 

 id:1355688997-19164-1-git-send-email-da...@tethera.net

Actually the first patch in the series is now an unrelated bug fix, 
since it isn't needed anymore for 2 and 3.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] notmuch-restore: fix return value propagation

2012-12-16 Thread david
From: David Bremner brem...@debian.org

Previously notmuch_restore_command returned 0 if tag_message returned
a non-zero (failure) value. This is wrong, since non-zero status
indicates something mysterious went wrong with retrieving the message,
or applying it.

There was also a failure to check or propagate the return value from
tag_op_list_apply in tag_message.
---
 notmuch-restore.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 40596a8..5a02328 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -25,6 +25,9 @@
 
 static regex_t regex;
 
+/* Non-zero return indicates an error in retrieving the message,
+ * or in applying the tags.
+ */
 static int
 tag_message (unused (void *ctx),
 notmuch_database_t *notmuch,
@@ -48,7 +51,7 @@ tag_message (unused (void *ctx),
 /* In order to detect missing messages, this check/optimization is
  * intentionally done *after* first finding the message. */
 if ((flags  TAG_FLAG_REMOVE_ALL) || tag_op_list_size (tag_ops))
-   tag_op_list_apply (message, tag_ops, flags);
+   ret = tag_op_list_apply (message, tag_ops, flags);
 
 notmuch_message_destroy (message);
 
@@ -231,8 +234,12 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
if (ret  0)
continue;
 
-   if (ret  0 || tag_message (ctx, notmuch, query_string,
-   tag_ops, flags))
+   if (ret  0)
+   break;
+
+   ret = tag_message (line_ctx, notmuch, query_string,
+  tag_ops, flags);
+   if (ret)
break;
 
 }  while ((line_len = getline (line, line_size, input)) != -1);
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 665373f..9ed9b51 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;
 
@@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;
 
+   if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, query_string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  query_string, tag_ops);
 
if (ret == 0) {
@@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 
 }  while ((line_len = getline (line, line_size, input)) != -1);
 
+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree (regex);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.

2012-12-16 Thread david
From: David Bremner brem...@debian.org

This code is no less correct than the previous version, since it does
not make sense for the array to live longer than the wrapping struct.

By not relying on the context passed into tag_parse_line, we can allow
tag_op_list_t structures to live longer than that context.
---
 notmuch-restore.c |2 +-
 tag-util.c|9 -
 tag-util.h|3 +--
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 5a02328..665373f 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -105,7 +105,7 @@ parse_sup_line (void *ctx, char *line,
tok_len++;
}
 
-   if (tag_op_list_append (ctx, tag_ops, tok, FALSE))
+   if (tag_op_list_append (tag_ops, tok, FALSE))
return -1;
 }
 
diff --git a/tag-util.c b/tag-util.c
index eab482f..705b7ba 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line,
goto DONE;
}
 
-   if (tag_op_list_append (ctx, tag_ops, tag, remove)) {
+   if (tag_op_list_append (tag_ops, tag, remove)) {
ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error,
  aborting);
goto DONE;
@@ -294,7 +294,7 @@ tag_op_list_create (void *ctx)
 list-size = TAG_OP_LIST_INITIAL_SIZE;
 list-count = 0;
 
-list-ops = talloc_array (ctx, tag_operation_t, list-size);
+list-ops = talloc_array (list, tag_operation_t, list-size);
 if (list-ops == NULL)
return NULL;
 
@@ -303,8 +303,7 @@ tag_op_list_create (void *ctx)
 
 
 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove)
 {
@@ -314,7 +313,7 @@ tag_op_list_append (void *ctx,
 
 if (list-count == list-size) {
list-size *= 2;
-   list-ops = talloc_realloc (ctx, list-ops, tag_operation_t,
+   list-ops = talloc_realloc (list, list-ops, tag_operation_t,
list-size);
if (list-ops == NULL) {
fprintf (stderr, Out of memory.\n);
diff --git a/tag-util.h b/tag-util.h
index 99b0fa0..c07bfde 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -87,8 +87,7 @@ tag_op_list_create (void *ctx);
  */
 
 int
-tag_op_list_append (void *ctx,
-   tag_op_list_t *list,
+tag_op_list_append (tag_op_list_t *list,
const char *tag,
notmuch_bool_t remove);
 
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch