Re: Xapian-quoting based batch-tagging.

2012-12-26 Thread Mark Walters

On Tue, 25 Dec 2012, da...@tethera.net wrote:
> This is an alternative version of 
>
>  id:1356313183-9266-1-git-send-email-da...@tethera.net
>
> batch tagging patches rebased on top of 
>
>  id:1356415076-5692-1-git-send-email-amdra...@mit.edu
>
> This mainly consisted of removing 
>
>  [Patch v9 04/17] notmuch-tag: factor out double quoting routine
>  (superceded by one of Austin's patches)
>
>  [Patch v9 05/17] util/string-util: add a new string tokenized function
>  [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded 
> queries
>  [Patch v9 07/17] notmuch-restore: move query handling for batch  
>  (uneeded if query is passed verbatim to xapian)
>
>  I also removed two tests, since they are about how we handle
>  quoting:
>
>  [Patch v9 13/17] test/tagging: add test for compound queries with batch 
> tagging
>  [Patch v9 17/17] test/tagging: add test for handling of parenthesized
> tag queries.
>
> A few small fixes were needed to the tests, and a fair amount of
> changes to the notmuch-tag man page.

Hi

I am unclear about how this is going to deal with queries containing
newlines. For dump/restore I think this is not a problem (as Austin and
others have said), but for batch tagging I think it could be; for
example the query could be for a tag containing a newline.

Best wishes

Mark


>
> Diffstat (against Austin's series) is as follows
>
>  man/man1/notmuch-tag.1 |   99 -
>  notmuch-tag.c  |  169 
>  tag-util.c |   87 +--
>  tag-util.h |   15 
>  test/tagging   |  195 ++
>  5 files changed, 480 insertions(+), 85 deletions(-)
>
>
> ___
> 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: Xapian-quoting based batch-tagging.

2012-12-26 Thread David Bremner
Mark Walters  writes:

> I am unclear about how this is going to deal with queries containing
> newlines. For dump/restore I think this is not a problem (as Austin and
> others have said), but for batch tagging I think it could be; for
> example the query could be for a tag containing a newline.

Yes, that's true, this patch series does not support queries with tags
with embedded newlines. They can still be removed (and added) via either
batch tagging or the command line. We could just live with this, or

- The current syntax allows for detecting options at the start of the
  line; perhaps a future fix would be to have the batch tagging and
  command line tagging accept an optionally hex encoded query, something
  like:

--hex +found%20it -- tag:%22stupid%0Atag%22

- Alternatively, we could add hex decoding on top of xapian quoting for
  queries. One UI downside is that people have to remember that % are
  special.

 +found%25it -- tag:lost%25it

  Another is that quoting is still (surprisingly) necessary for encoded
  spaces
  
 +found%20it -- tag:"lost%20it"
 
  Introducing yet another escape format, e.g. "\n" would require more
  code, and not really much benefit afaict versus re-using hex-encoding.
  Offhand I don't see how to avoid this without some level of query
  pre-processing a-la
  
id:1356313183-9266-1-git-send-email-da...@tethera.net

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


Re: [PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference

2012-12-26 Thread David Bremner
da...@tethera.net writes:

> From: David Bremner 
>
> Consistently use [...]; one less space. Use singular 
> ---

pushed this one patch.

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


Re: [PATCH 01/11] parse_tag_line: use enum for return value.

2012-12-26 Thread David Bremner
da...@tethera.net writes:

> From: David Bremner 
>
> This is essentially cosmetic, since success=0 is promised by
> the comments in tag-utils.h.

Pushed this one patch.

Both of the two I pushed from this series are really nothing to do with
batch tagging, and were just cluttering things up.

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


Re: [PATCH v2 1/5] util: Factor out boolean term quoting routine

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

> +/* Construct a boolean term query with the specified prefix (e.g.,
> + * "id") and search term, quoting term as necessary.
> + *
> + * Output is into buf; it may be talloc_realloced.
> + * Return: 0 on success, non-zero on memory allocation failure.
> + */
> +int make_boolean_term (void *talloc_ctx, const char *prefix, const char 
> *term,
> +char **buf, size_t *len);
> +

I think the quoting should be described a bit more precisely here,
especially since you are going to refer to it in the comment above
parse_boolean_term.

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


Re: [PATCH v2 2/5] util: Function to parse boolean term queries

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

> + char *out = talloc_strdup (ctx, pos + 1);
> + int closed = 0;
> + /* Find the closing quote and un-double doubled internal
> +  * quotes. */
> + for (pos = *term_out = out; *pos; ) {

Since you strdup anyway, wouldn't it be easier to understand if pos read
from the input string and out wrote to term_out? Something like
(untested) 

index db01b4b..e4157d0 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -112,11 +112,12 @@ parse_boolean_term (void *ctx, const char *str,
 
 /* Implement de-quoting compatible with make_boolean_term. */
 if (*pos == '"') {
-   char *out = talloc_strdup (ctx, pos + 1);
+   char *out;
int closed = 0;
+   *term_out= talloc_strdup (ctx, pos + 1);
/* Find the closing quote and un-double doubled internal
 * quotes. */
-   for (pos = *term_out = out; *pos; ) {
+   for (out = *term_out; *pos; ) {
if (*pos == '"') {
++pos;


Perhaps the two talloc_strdups can even be unified, but I wouldn't worry
too much about that.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 0/5] Use Xapian query syntax for batch-tag dump/restore

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

> This obsoletes
>
>   id:1356415076-5692-1-git-send-email-amdra...@mit.edu
>
> In addition to incorporating all of David's suggestions, this reworks
> the boolean term parsing so it only handles the subset of quoting
> syntax used by make_boolean_term (which also happens to be all that we
> described in the man page for the format).  The diff from v1 is below.

Implementation wise this series now looks OK to me; I think the dequote
routine could be slightly clearer, but it's a bit subjective at this
point.

IMHO the real question is if we like (or can live with) the resulting UI
for batch tagging.  Mark ( id:87ehid5h64@qmul.ac.uk ) brings up the
question of whether tags with newlines should be somehow supported in a
batch-tagging context. The other UI issue I'm aware of is the fact that
tags with spaces needed to be quoted in two different ways for tag
operations and for queries.  A relatively smooth upgrade path would be
to support Xapian quoting for tag operations in the future.

d

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


Re: [PATCH] emacs: show: make id links respect window

2012-12-26 Thread David Bremner
Mark Walters  writes:

>> I can imagine that people would want/like the "open in other window" 
>> effect of the current code, even if the reason is a bug.
>
> That's definitely possible. I generally expect a mouse click to select
> the window I click and this feels counter intuitive. I think that some
> people might like an option "open this link in a new window" but I would
> guess that would like that whether they clicked or pressed RET on the
> button.

I don't care much either way myself, but before we change notmuch-show
behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
feedback from other people.

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


[PATCH] notmuch.c: run uncrustify

2012-12-26 Thread david
From: David Bremner 

In anticipation of doing some updates to this code, it simplifies life
if the code is "uncrustify clean" to start with
---
 notmuch.c |   23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 9516dfb..ee2892e 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -22,7 +22,7 @@
 
 #include "notmuch-client.h"
 
-typedef int (*command_function_t) (void *ctx, int argc, char *argv[]);
+typedef int (*command_function_t)(void *ctx, int argc, char *argv[]);
 
 typedef struct command {
 const char *name;
@@ -39,8 +39,8 @@ typedef struct alias {
 } alias_t;
 
 alias_t aliases[] = {
-{ "part", { "show", "--format=raw"}},
-{ "search-tags", {"search", "--output=tags", "*"}}
+{ "part", { "show", "--format=raw" } },
+{ "search-tags", { "search", "--output=tags", "*" } }
 };
 
 static int
@@ -66,7 +66,7 @@ static command_t commands[] = {
   "[options...]  [...]",
   "Construct a reply template for a set of messages." },
 { "tag", notmuch_tag_command,
-  "+|- [...] [--]  [...]" ,
+  "+|- [...] [--]  [...]",
   "Add/remove tags for all messages matching the search terms." },
 { "dump", notmuch_dump_command,
   "[] [--] []",
@@ -107,8 +107,8 @@ usage (FILE *out)
 
 fprintf (out, "\n");
 fprintf (out,
-"Use \"notmuch help \" for more details on each command\n"
-"and \"notmuch help search-terms\" for the common search-terms 
syntax.\n\n");
+"Use \"notmuch help \" for more details on each command\n"
+"and \"notmuch help search-terms\" for the common search-terms 
syntax.\n\n");
 }
 
 void
@@ -281,15 +281,14 @@ main (int argc, char *argv[])
return notmuch_help_command (NULL, argc - 1, &argv[1]);
 
 if (strcmp (argv[1], "--version") == 0) {
-   printf ("notmuch " STRINGIFY(NOTMUCH_VERSION) "\n");
+   printf ("notmuch " STRINGIFY (NOTMUCH_VERSION) "\n");
return 0;
 }
 
 for (i = 0; i < ARRAY_SIZE (aliases); i++) {
alias = &aliases[i];
 
-   if (strcmp (argv[1], alias->name) == 0)
-   {
+   if (strcmp (argv[1], alias->name) == 0) {
int substitutions;
 
argv_local = talloc_size (local, sizeof (char *) *
@@ -304,14 +303,14 @@ main (int argc, char *argv[])
for (j = 0; j < MAX_ALIAS_SUBSTITUTIONS; j++) {
if (alias->substitutions[j] == NULL)
break;
-   argv_local[j+1] = alias->substitutions[j];
+   argv_local[j + 1] = alias->substitutions[j];
}
substitutions = j;
 
/* And copy all original arguments (skipping the argument
 * that matched the alias of course. */
for (j = 2; j < (unsigned) argc; j++) {
-   argv_local[substitutions+j-1] = argv[j];
+   argv_local[substitutions + j - 1] = argv[j];
}
 
argc += substitutions - 1;
@@ -323,7 +322,7 @@ main (int argc, char *argv[])
command = &commands[i];
 
if (strcmp (argv[1], command->name) == 0)
-   return (command->function) (local, argc - 1, &argv[1]);
+   return (command->function)(local, argc - 1, &argv[1]);
 }
 
 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 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace.

2012-12-26 Thread david
From: David Bremner 

Three of these are marked broken; the third is a regression test,
since it passes by virtue of batch-tag being the default input format.
---
 test/dump-restore |   42 ++
 1 file changed, 42 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index 6a989b6..c2ddb92 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -136,6 +136,48 @@ notmuch dump --format=batch-tag > BACKUP
 
 notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*"
 
+# initial segment of file used for several tests below.
+cat < comments-and-blanks
+# this is a comment
+
+# next line has leading whitespace
+   
+
+EOF
+
+test_begin_subtest 'restoring empty file is not an error'
+test_subtest_known_broken
+notmuch restore < /dev/null 2>OUTPUT.$test_count
+cp /dev/null EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
+test_begin_subtest 'file of comments and blank lines is not an error'
+test_subtest_known_broken
+notmuch restore --input=comments-and-blanks
+ret_val=$?
+test_expect_equal "$ret_val" "0"
+
+cp comments-and-blanks leading-comments-blanks-batch-tag
+echo "+some_tag -- id:yun1vjwegii@aiko.keithp.com" \
+>> leading-comments-blanks-batch-tag
+
+test_begin_subtest 'detect format=batch-tag with leading comments and blanks'
+notmuch restore --input=leading-comments-blanks-batch-tag
+notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > 
OUTPUT.$test_count
+echo "some_tag" > EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
+cp comments-and-blanks leading-comments-blanks-sup
+echo "yun1vjwegii@aiko.keithp.com (another_tag)" \
+>> leading-comments-blanks-sup
+
+test_begin_subtest 'detect format=sup with leading comments and blanks'
+test_subtest_known_broken
+notmuch restore --input=leading-comments-blanks-sup
+notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > 
OUTPUT.$test_count
+echo "another_tag" > EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
 test_begin_subtest 'format=batch-tag, round trip with strange tags'
 notmuch dump --format=batch-tag > EXPECTED.$test_count
 notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
-- 
1.7.10.4

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


[PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments.

2012-12-26 Thread david
From: David Bremner 

This patch corrects several undesirable behaviours:

1) Empty files were not detected, leading to buffer read overrun.

2) An initial blank line cause restore to silently abort

3) Initial comment line caused format detection to fail
---
 notmuch-restore.c |   17 -
 test/dump-restore |3 ---
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 9ed9b51..c93f1ac 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 argv[opt_index]);
return 1;
 }
-char *p;
-
-line_len = getline (&line, &line_size, input);
-if (line_len == 0)
-   return 0;
 
 tag_ops = tag_op_list_create (ctx);
 if (tag_ops == NULL) {
@@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
return 1;
 }
 
+do {
+   line_len = getline (&line, &line_size, input);
+
+   /* empty input file not considered an error */
+   if (line_len < 0)
+   return 0;
+
+} while ((line_len == 0) ||
+(line[0] == '#') ||
+(strspn (line, " \t\n") == strlen (line)));
+
+char *p;
 for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) {
if (*p == '(')
input_format = DUMP_FORMAT_SUP;
diff --git a/test/dump-restore b/test/dump-restore
index c2ddb92..ae30cd1 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -146,13 +146,11 @@ cat < comments-and-blanks
 EOF
 
 test_begin_subtest 'restoring empty file is not an error'
-test_subtest_known_broken
 notmuch restore < /dev/null 2>OUTPUT.$test_count
 cp /dev/null EXPECTED
 test_expect_equal_file EXPECTED OUTPUT.$test_count
 
 test_begin_subtest 'file of comments and blank lines is not an error'
-test_subtest_known_broken
 notmuch restore --input=comments-and-blanks
 ret_val=$?
 test_expect_equal "$ret_val" "0"
@@ -172,7 +170,6 @@ echo "yun1vjwegii@aiko.keithp.com (another_tag)" \
 >> leading-comments-blanks-sup
 
 test_begin_subtest 'detect format=sup with leading comments and blanks'
-test_subtest_known_broken
 notmuch restore --input=leading-comments-blanks-sup
 notmuch search --output=tags id:yun1vjwegii@aiko.keithp.com > 
OUTPUT.$test_count
 echo "another_tag" > EXPECTED
-- 
1.7.10.4

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


v2 talloc leak report

2012-12-26 Thread david

Obsoletes 

  id:1355714648-23144-1-git-send-email-da...@tethera.netz

[Patch v2 1/4] CLI: add talloc leak report, controlled by an
- comments and formatting

[Patch v2 2/4] util: add talloc-extra.[ch]
- rename, fix dangerous memcpy

[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup
no change, except new names

[Patch v2 4/4] perf-test: initial support for talloc leak report in
New patch, use talloc reports in memory tests.

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


[Patch v2 2/4] util: add talloc-extra.[ch]

2012-12-26 Thread david
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/talloc-extra.c |   14 ++
 util/talloc-extra.h |   18 ++
 4 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 util/talloc-extra.c
 create mode 100644 util/talloc-extra.h

diff --git a/notmuch-client.h b/notmuch-client.h
index d7b352e..5f28836 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include 
 #include 
 
-#include 
+#include "talloc-extra.h"
 
 #define unused(x) x __attribute__ ((unused))
 
diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..29c0ce6 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)/talloc-extra.c
 
 libutil_modules := $(libutil_c_srcs:.c=.o)
 
diff --git a/util/talloc-extra.c b/util/talloc-extra.c
new file mode 100644
index 000..4a5f9c0
--- /dev/null
+++ b/util/talloc-extra.c
@@ -0,0 +1,14 @@
+#include 
+#include "talloc-extra.h"
+
+char *
+talloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_strndup (ctx, str, len);
+
+if (ptr)
+   talloc_set_name_const(ptr, name);
+
+return ptr;
+}
diff --git a/util/talloc-extra.h b/util/talloc-extra.h
new file mode 100644
index 000..5b8ca28
--- /dev/null
+++ b/util/talloc-extra.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 *
+talloc_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 talloc_strndup_debug(ctx, str, len) talloc_strndup_named_const (ctx, 
str, len, __location__)
+
+#endif
-- 
1.7.10.4

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


[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup

2012-12-26 Thread david
From: David Bremner 

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

diff --git a/notmuch-restore.c b/notmuch-restore.c
index c93f1ac..6111977 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -88,10 +88,11 @@ 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,
-   match[2].rm_eo - match[2].rm_so);
+*query_str = talloc_strndup_debug (ctx, line + match[1].rm_so,
+  match[1].rm_eo - match[1].rm_so);
+
+file_tags = talloc_strndup_debug (ctx, line + match[2].rm_so,
+ match[2].rm_eo - match[2].rm_so);
 
 char *tok = file_tags;
 size_t tok_len = 0;
-- 
1.7.10.4

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


[Patch v2 4/4] perf-test: initial support for talloc leak report in memory tests

2012-12-26 Thread david
From: David Bremner 

As with the valgrind logs, we print a (very) brief summary and leave
the log for inspection.
---
 performance-test/perf-test-lib.sh |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/performance-test/perf-test-lib.sh 
b/performance-test/perf-test-lib.sh
index 10d05e0..9ee7661 100644
--- a/performance-test/perf-test-lib.sh
+++ b/performance-test/perf-test-lib.sh
@@ -126,13 +126,16 @@ memory_run ()
 test_count=$(($test_count+1))
 
 log_file=$log_dir/$test_count.log
+talloc_log=$log_dir/$test_count.talloc
 
 printf "[ %d ]\t%s\n" $test_count "$1"
 
-valgrind --leak-check=full --log-file="$log_file" $2
+NOTMUCH_TALLOC_REPORT="$talloc_log" valgrind --leak-check=full 
--log-file="$log_file" $2
 
 awk '/LEAK SUMMARY/,/suppressed/ { sub(/^==[0-9]*==/," "); print }' 
"$log_file"
 echo
+sed -n -e 's/.*[(]total *\([^)]*\)[)]/talloced at exit: \1/p' $talloc_log
+echo
 }
 
 memory_done ()
-- 
1.7.10.4

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


[Patch v2 1/4] CLI: add talloc leak report, controlled by an environment variable.

2012-12-26 Thread david
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 |   24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index ee2892e..6b7aae8 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -321,8 +321,28 @@ 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]);
+
+   /* in the future support for this environment variable may
+* be supplemented or replaced by command line arguments
+* --leak-report and/or --leak-report-full */
+
+   talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
+
+   /* this relies on the previous call to
+* talloc_enable_null_tracking */
+
+   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


Re: [PATCH] emacs: tweak error buffer handling

2012-12-26 Thread Mark Walters

On Tue, 25 Dec 2012, Tomi Ollila  wrote:
> On Sat, Dec 22 2012, Mark Walters  wrote:
>
>> view-mode-enter changed between emacs 23 and emacs 24: the current
>> code makes the error buffer disappear in emacs 24 on quitting it (ie
>> pressing q) but this just kills the buffer without closing the split
>> window in emacs 23.
>>
>> This patch makes the error buffer window disappear in emacs 23
>> too. Since the view-mode-enter function changed we have to test for
>> version and do the correct thing in each case.
>> ---
>>
>> This seems to work but I have only tested on 23.4 and 24.2
>
> I run emacs 23.1.1 to get the documentation of view-mode-enter
> there. So, this patch instructs to delete WINDOW when exiting
> view mode...
>
> Documentation of pop-to-buffer says:
>
> "Select buffer BUFFER-OR-NAME in some window, preferably a different one."
>
> What if pop-up-windows's value is nil -- the content of current window
> is replaced with this view stuff -- and when exiting view mode, the
> window will be deleted ? What happens with emacs 24 in this case ?

Hi 

You are quite right there are problems here under emacs 23: if you
already have a split window when the error occurs in one part the error
is displayed in the other window and then on exit that (previously
existing) window is closed.

What do people think should happen on an error? I, personally, don't
like taking over an existing window, and Jamie liked some of the errors
(eg non-fatal `locked database' tagging errors) to be shown in the
mini-buffer.

I also think it is going to be difficult to get this right: emacs 23 and
24 are different and there are also some user configuration variable
that affect what happens.

Best wishes

Mark



>
> Tomi
>
>> Best wishes
>>
>> Mark
>>
>>
>>
>>  emacs/notmuch-lib.el |8 +---
>>  1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 77a591d..0407f8a 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -324,15 +324,17 @@ the user dismisses it."
>>  
>>(let ((buf (get-buffer-create "*Notmuch errors*")))
>>  (with-current-buffer buf
>> -  (view-mode-enter nil #'kill-buffer)
>> +  (pop-to-buffer buf)
>> +  (view-mode-enter (when (< emacs-major-version 24)
>> +   (cons (selected-window) (cons nil t)))
>> +   #'kill-buffer)
>>(let ((inhibit-read-only t))
>>  (goto-char (point-max))
>>  (unless (bobp)
>>(insert "\n"))
>>  (insert msg)
>>  (unless (bolp)
>> -  (insert "\n"
>> -(pop-to-buffer buf)))
>> +  (insert "\n"))
>>  
>>  (defun notmuch-check-async-exit-status (proc msg)
>>"If PROC exited abnormally, pop up an error buffer and signal an error.
>> -- 
>> 1.7.9.1
>>
>> ___
>> 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: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-12-26 Thread David Bremner
Michael Forney  writes:

> On Thu, 18 Oct 2012 00:52:20 +0300, Adrien Bustany  wrote:
>> The code of the patches in unchanged, but the formatting issues are now
>> hopefully fixed.
>
> I would like to bump this patch set. I also need these features from
> libnotmuch. Currently there is no way to recover from Xapian errors,
> which is pretty limiting.
>
> If it is the flush/commit that is the issue, I would be happy to make an
> updated patch set.

Hi Michael;

Right, we should use the current name of 'commit'. We also need some way
to check the xapian version in the configure script, and to make sure
the right name is supported.

Jani refers to 

 id:1350487737-32058-2-git-send-email-bgamari.f...@gmail.com

farther up the thread as having some relevant code/ideas.

I think it's reasonable to require Xapian version at least 1.2.0, so
that might simplify the configuration check. 

The other thing I wondered is if the error handling in these routines is
as good as it could be. I guess it isn't worse than what is there, but
especially since you are explicitly wanting to handle error conditions,
it would be good to think if we can do something better than adding yet
another printf to the library.

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


Xapian-quoting based batch-tagging.

2012-12-26 Thread Mark Walters

On Tue, 25 Dec 2012, david at tethera.net wrote:
> This is an alternative version of 
>
>  id:1356313183-9266-1-git-send-email-david at tethera.net
>
> batch tagging patches rebased on top of 
>
>  id:1356415076-5692-1-git-send-email-amdragon at mit.edu
>
> This mainly consisted of removing 
>
>  [Patch v9 04/17] notmuch-tag: factor out double quoting routine
>  (superceded by one of Austin's patches)
>
>  [Patch v9 05/17] util/string-util: add a new string tokenized function
>  [Patch v9 06/17] unhex_and_quote: new function to quote hex-decoded 
> queries
>  [Patch v9 07/17] notmuch-restore: move query handling for batch  
>  (uneeded if query is passed verbatim to xapian)
>
>  I also removed two tests, since they are about how we handle
>  quoting:
>
>  [Patch v9 13/17] test/tagging: add test for compound queries with batch 
> tagging
>  [Patch v9 17/17] test/tagging: add test for handling of parenthesized
> tag queries.
>
> A few small fixes were needed to the tests, and a fair amount of
> changes to the notmuch-tag man page.

Hi

I am unclear about how this is going to deal with queries containing
newlines. For dump/restore I think this is not a problem (as Austin and
others have said), but for batch tagging I think it could be; for
example the query could be for a tag containing a newline.

Best wishes

Mark


>
> Diffstat (against Austin's series) is as follows
>
>  man/man1/notmuch-tag.1 |   99 -
>  notmuch-tag.c  |  169 
>  tag-util.c |   87 +--
>  tag-util.h |   15 
>  test/tagging   |  195 ++
>  5 files changed, 480 insertions(+), 85 deletions(-)
>
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Xapian-quoting based batch-tagging.

2012-12-26 Thread David Bremner
Mark Walters  writes:

> I am unclear about how this is going to deal with queries containing
> newlines. For dump/restore I think this is not a problem (as Austin and
> others have said), but for batch tagging I think it could be; for
> example the query could be for a tag containing a newline.

Yes, that's true, this patch series does not support queries with tags
with embedded newlines. They can still be removed (and added) via either
batch tagging or the command line. We could just live with this, or

- The current syntax allows for detecting options at the start of the
  line; perhaps a future fix would be to have the batch tagging and
  command line tagging accept an optionally hex encoded query, something
  like:

--hex +found%20it -- tag:%22stupid%0Atag%22

- Alternatively, we could add hex decoding on top of xapian quoting for
  queries. One UI downside is that people have to remember that % are
  special.

 +found%25it -- tag:lost%25it

  Another is that quoting is still (surprisingly) necessary for encoded
  spaces

 +found%20it -- tag:"lost%20it"

  Introducing yet another escape format, e.g. "\n" would require more
  code, and not really much benefit afaict versus re-using hex-encoding.
  Offhand I don't see how to avoid this without some level of query
  pre-processing a-la

id:1356313183-9266-1-git-send-email-david at tethera.net

d


[PATCH 09/11] notmuch-tag.1: tidy synopsis formatting, reference

2012-12-26 Thread David Bremner
david at tethera.net writes:

> From: David Bremner 
>
> Consistently use [...]; one less space. Use singular 
> ---

pushed this one patch.

d


[PATCH 01/11] parse_tag_line: use enum for return value.

2012-12-26 Thread David Bremner
david at tethera.net writes:

> From: David Bremner 
>
> This is essentially cosmetic, since success=0 is promised by
> the comments in tag-utils.h.

Pushed this one patch.

Both of the two I pushed from this series are really nothing to do with
batch tagging, and were just cluttering things up.

d


[PATCH v2 1/5] util: Factor out boolean term quoting routine

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

> +/* Construct a boolean term query with the specified prefix (e.g.,
> + * "id") and search term, quoting term as necessary.
> + *
> + * Output is into buf; it may be talloc_realloced.
> + * Return: 0 on success, non-zero on memory allocation failure.
> + */
> +int make_boolean_term (void *talloc_ctx, const char *prefix, const char 
> *term,
> +char **buf, size_t *len);
> +

I think the quoting should be described a bit more precisely here,
especially since you are going to refer to it in the comment above
parse_boolean_term.

d


[PATCH v2 2/5] util: Function to parse boolean term queries

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

> + char *out = talloc_strdup (ctx, pos + 1);
> + int closed = 0;
> + /* Find the closing quote and un-double doubled internal
> +  * quotes. */
> + for (pos = *term_out = out; *pos; ) {

Since you strdup anyway, wouldn't it be easier to understand if pos read
from the input string and out wrote to term_out? Something like
(untested) 

index db01b4b..e4157d0 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -112,11 +112,12 @@ parse_boolean_term (void *ctx, const char *str,

 /* Implement de-quoting compatible with make_boolean_term. */
 if (*pos == '"') {
-   char *out = talloc_strdup (ctx, pos + 1);
+   char *out;
int closed = 0;
+   *term_out= talloc_strdup (ctx, pos + 1);
/* Find the closing quote and un-double doubled internal
 * quotes. */
-   for (pos = *term_out = out; *pos; ) {
+   for (out = *term_out; *pos; ) {
if (*pos == '"') {
++pos;


Perhaps the two talloc_strdups can even be unified, but I wouldn't worry
too much about that.


[PATCH v2 0/5] Use Xapian query syntax for batch-tag dump/restore

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

> This obsoletes
>
>   id:1356415076-5692-1-git-send-email-amdragon at mit.edu
>
> In addition to incorporating all of David's suggestions, this reworks
> the boolean term parsing so it only handles the subset of quoting
> syntax used by make_boolean_term (which also happens to be all that we
> described in the man page for the format).  The diff from v1 is below.

Implementation wise this series now looks OK to me; I think the dequote
routine could be slightly clearer, but it's a bit subjective at this
point.

IMHO the real question is if we like (or can live with) the resulting UI
for batch tagging.  Mark ( id:87ehid5h64.fsf at qmul.ac.uk ) brings up the
question of whether tags with newlines should be somehow supported in a
batch-tagging context. The other UI issue I'm aware of is the fact that
tags with spaces needed to be quoted in two different ways for tag
operations and for queries.  A relatively smooth upgrade path would be
to support Xapian quoting for tag operations in the future.

d



[PATCH] emacs: show: make id links respect window

2012-12-26 Thread David Bremner
Mark Walters  writes:

>> I can imagine that people would want/like the "open in other window" 
>> effect of the current code, even if the reason is a bug.
>
> That's definitely possible. I generally expect a mouse click to select
> the window I click and this feels counter intuitive. I think that some
> people might like an option "open this link in a new window" but I would
> guess that would like that whether they clicked or pressed RET on the
> button.

I don't care much either way myself, but before we change notmuch-show
behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
feedback from other people.

d


[PATCH] notmuch.c: run uncrustify

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

In anticipation of doing some updates to this code, it simplifies life
if the code is "uncrustify clean" to start with
---
 notmuch.c |   23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 9516dfb..ee2892e 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -22,7 +22,7 @@

 #include "notmuch-client.h"

-typedef int (*command_function_t) (void *ctx, int argc, char *argv[]);
+typedef int (*command_function_t)(void *ctx, int argc, char *argv[]);

 typedef struct command {
 const char *name;
@@ -39,8 +39,8 @@ typedef struct alias {
 } alias_t;

 alias_t aliases[] = {
-{ "part", { "show", "--format=raw"}},
-{ "search-tags", {"search", "--output=tags", "*"}}
+{ "part", { "show", "--format=raw" } },
+{ "search-tags", { "search", "--output=tags", "*" } }
 };

 static int
@@ -66,7 +66,7 @@ static command_t commands[] = {
   "[options...]  [...]",
   "Construct a reply template for a set of messages." },
 { "tag", notmuch_tag_command,
-  "+|- [...] [--]  [...]" ,
+  "+|- [...] [--]  [...]",
   "Add/remove tags for all messages matching the search terms." },
 { "dump", notmuch_dump_command,
   "[] [--] []",
@@ -107,8 +107,8 @@ usage (FILE *out)

 fprintf (out, "\n");
 fprintf (out,
-"Use \"notmuch help \" for more details on each command\n"
-"and \"notmuch help search-terms\" for the common search-terms 
syntax.\n\n");
+"Use \"notmuch help \" for more details on each command\n"
+"and \"notmuch help search-terms\" for the common search-terms 
syntax.\n\n");
 }

 void
@@ -281,15 +281,14 @@ main (int argc, char *argv[])
return notmuch_help_command (NULL, argc - 1, &argv[1]);

 if (strcmp (argv[1], "--version") == 0) {
-   printf ("notmuch " STRINGIFY(NOTMUCH_VERSION) "\n");
+   printf ("notmuch " STRINGIFY (NOTMUCH_VERSION) "\n");
return 0;
 }

 for (i = 0; i < ARRAY_SIZE (aliases); i++) {
alias = &aliases[i];

-   if (strcmp (argv[1], alias->name) == 0)
-   {
+   if (strcmp (argv[1], alias->name) == 0) {
int substitutions;

argv_local = talloc_size (local, sizeof (char *) *
@@ -304,14 +303,14 @@ main (int argc, char *argv[])
for (j = 0; j < MAX_ALIAS_SUBSTITUTIONS; j++) {
if (alias->substitutions[j] == NULL)
break;
-   argv_local[j+1] = alias->substitutions[j];
+   argv_local[j + 1] = alias->substitutions[j];
}
substitutions = j;

/* And copy all original arguments (skipping the argument
 * that matched the alias of course. */
for (j = 2; j < (unsigned) argc; j++) {
-   argv_local[substitutions+j-1] = argv[j];
+   argv_local[substitutions + j - 1] = argv[j];
}

argc += substitutions - 1;
@@ -323,7 +322,7 @@ main (int argc, char *argv[])
command = &commands[i];

if (strcmp (argv[1], command->name) == 0)
-   return (command->function) (local, argc - 1, &argv[1]);
+   return (command->function)(local, argc - 1, &argv[1]);
 }

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



[PATCH 1/2] test/dump-restore: new tests for empty files and leading comments/whitespace.

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

Three of these are marked broken; the third is a regression test,
since it passes by virtue of batch-tag being the default input format.
---
 test/dump-restore |   42 ++
 1 file changed, 42 insertions(+)

diff --git a/test/dump-restore b/test/dump-restore
index 6a989b6..c2ddb92 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -136,6 +136,48 @@ notmuch dump --format=batch-tag > BACKUP

 notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*"

+# initial segment of file used for several tests below.
+cat < comments-and-blanks
+# this is a comment
+
+# next line has leading whitespace
+   
+
+EOF
+
+test_begin_subtest 'restoring empty file is not an error'
+test_subtest_known_broken
+notmuch restore < /dev/null 2>OUTPUT.$test_count
+cp /dev/null EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
+test_begin_subtest 'file of comments and blank lines is not an error'
+test_subtest_known_broken
+notmuch restore --input=comments-and-blanks
+ret_val=$?
+test_expect_equal "$ret_val" "0"
+
+cp comments-and-blanks leading-comments-blanks-batch-tag
+echo "+some_tag -- id:yun1vjwegii.fsf at aiko.keithp.com" \
+>> leading-comments-blanks-batch-tag
+
+test_begin_subtest 'detect format=batch-tag with leading comments and blanks'
+notmuch restore --input=leading-comments-blanks-batch-tag
+notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > 
OUTPUT.$test_count
+echo "some_tag" > EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
+cp comments-and-blanks leading-comments-blanks-sup
+echo "yun1vjwegii.fsf at aiko.keithp.com (another_tag)" \
+>> leading-comments-blanks-sup
+
+test_begin_subtest 'detect format=sup with leading comments and blanks'
+test_subtest_known_broken
+notmuch restore --input=leading-comments-blanks-sup
+notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > 
OUTPUT.$test_count
+echo "another_tag" > EXPECTED
+test_expect_equal_file EXPECTED OUTPUT.$test_count
+
 test_begin_subtest 'format=batch-tag, round trip with strange tags'
 notmuch dump --format=batch-tag > EXPECTED.$test_count
 notmuch dump --format=batch-tag | notmuch restore --format=batch-tag
-- 
1.7.10.4



[PATCH 2/2] notmuch-restore: handle empty input file, leading blank lines and comments.

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

This patch corrects several undesirable behaviours:

1) Empty files were not detected, leading to buffer read overrun.

2) An initial blank line cause restore to silently abort

3) Initial comment line caused format detection to fail
---
 notmuch-restore.c |   17 -
 test/dump-restore |3 ---
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 9ed9b51..c93f1ac 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -180,11 +180,6 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 argv[opt_index]);
return 1;
 }
-char *p;
-
-line_len = getline (&line, &line_size, input);
-if (line_len == 0)
-   return 0;

 tag_ops = tag_op_list_create (ctx);
 if (tag_ops == NULL) {
@@ -192,6 +187,18 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
return 1;
 }

+do {
+   line_len = getline (&line, &line_size, input);
+
+   /* empty input file not considered an error */
+   if (line_len < 0)
+   return 0;
+
+} while ((line_len == 0) ||
+(line[0] == '#') ||
+(strspn (line, " \t\n") == strlen (line)));
+
+char *p;
 for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) {
if (*p == '(')
input_format = DUMP_FORMAT_SUP;
diff --git a/test/dump-restore b/test/dump-restore
index c2ddb92..ae30cd1 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -146,13 +146,11 @@ cat < comments-and-blanks
 EOF

 test_begin_subtest 'restoring empty file is not an error'
-test_subtest_known_broken
 notmuch restore < /dev/null 2>OUTPUT.$test_count
 cp /dev/null EXPECTED
 test_expect_equal_file EXPECTED OUTPUT.$test_count

 test_begin_subtest 'file of comments and blank lines is not an error'
-test_subtest_known_broken
 notmuch restore --input=comments-and-blanks
 ret_val=$?
 test_expect_equal "$ret_val" "0"
@@ -172,7 +170,6 @@ echo "yun1vjwegii.fsf at aiko.keithp.com (another_tag)" \
 >> leading-comments-blanks-sup

 test_begin_subtest 'detect format=sup with leading comments and blanks'
-test_subtest_known_broken
 notmuch restore --input=leading-comments-blanks-sup
 notmuch search --output=tags id:yun1vjwegii.fsf at aiko.keithp.com > 
OUTPUT.$test_count
 echo "another_tag" > EXPECTED
-- 
1.7.10.4



v2 talloc leak report

2012-12-26 Thread da...@tethera.net

Obsoletes 

  id:1355714648-23144-1-git-send-email-david at tethera.netZ

[Patch v2 1/4] CLI: add talloc leak report, controlled by an
- comments and formatting

[Patch v2 2/4] util: add talloc-extra.[ch]
- rename, fix dangerous memcpy

[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup
no change, except new names

[Patch v2 4/4] perf-test: initial support for talloc leak report in
New patch, use talloc reports in memory tests.



[Patch v2 2/4] util: add talloc-extra.[ch]

2012-12-26 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/talloc-extra.c |   14 ++
 util/talloc-extra.h |   18 ++
 4 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 util/talloc-extra.c
 create mode 100644 util/talloc-extra.h

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

-#include 
+#include "talloc-extra.h"

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

diff --git a/util/Makefile.local b/util/Makefile.local
index a11e35b..29c0ce6 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)/talloc-extra.c

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

diff --git a/util/talloc-extra.c b/util/talloc-extra.c
new file mode 100644
index 000..4a5f9c0
--- /dev/null
+++ b/util/talloc-extra.c
@@ -0,0 +1,14 @@
+#include 
+#include "talloc-extra.h"
+
+char *
+talloc_strndup_named_const (void *ctx, const char *str,
+size_t len, const char *name)
+{
+char *ptr = talloc_strndup (ctx, str, len);
+
+if (ptr)
+   talloc_set_name_const(ptr, name);
+
+return ptr;
+}
diff --git a/util/talloc-extra.h b/util/talloc-extra.h
new file mode 100644
index 000..5b8ca28
--- /dev/null
+++ b/util/talloc-extra.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 *
+talloc_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 talloc_strndup_debug(ctx, str, len) talloc_strndup_named_const (ctx, 
str, len, __location__)
+
+#endif
-- 
1.7.10.4



[Patch v2 3/4] notmuch-restore: use debug version of talloc_strndup

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

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

diff --git a/notmuch-restore.c b/notmuch-restore.c
index c93f1ac..6111977 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -88,10 +88,11 @@ 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,
-   match[2].rm_eo - match[2].rm_so);
+*query_str = talloc_strndup_debug (ctx, line + match[1].rm_so,
+  match[1].rm_eo - match[1].rm_so);
+
+file_tags = talloc_strndup_debug (ctx, line + match[2].rm_so,
+ match[2].rm_eo - match[2].rm_so);

 char *tok = file_tags;
 size_t tok_len = 0;
-- 
1.7.10.4



[Patch v2 4/4] perf-test: initial support for talloc leak report in memory tests

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

As with the valgrind logs, we print a (very) brief summary and leave
the log for inspection.
---
 performance-test/perf-test-lib.sh |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/performance-test/perf-test-lib.sh 
b/performance-test/perf-test-lib.sh
index 10d05e0..9ee7661 100644
--- a/performance-test/perf-test-lib.sh
+++ b/performance-test/perf-test-lib.sh
@@ -126,13 +126,16 @@ memory_run ()
 test_count=$(($test_count+1))

 log_file=$log_dir/$test_count.log
+talloc_log=$log_dir/$test_count.talloc

 printf "[ %d ]\t%s\n" $test_count "$1"

-valgrind --leak-check=full --log-file="$log_file" $2
+NOTMUCH_TALLOC_REPORT="$talloc_log" valgrind --leak-check=full 
--log-file="$log_file" $2

 awk '/LEAK SUMMARY/,/suppressed/ { sub(/^==[0-9]*==/," "); print }' 
"$log_file"
 echo
+sed -n -e 's/.*[(]total *\([^)]*\)[)]/talloced at exit: \1/p' $talloc_log
+echo
 }

 memory_done ()
-- 
1.7.10.4



[Patch v2 1/4] CLI: add talloc leak report, controlled by an environment variable.

2012-12-26 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 |   24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index ee2892e..6b7aae8 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -321,8 +321,28 @@ 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]);
+
+   /* in the future support for this environment variable may
+* be supplemented or replaced by command line arguments
+* --leak-report and/or --leak-report-full */
+
+   talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
+
+   /* this relies on the previous call to
+* talloc_enable_null_tracking */
+
+   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



[PATCH] emacs: tweak error buffer handling

2012-12-26 Thread Mark Walters

On Tue, 25 Dec 2012, Tomi Ollila  wrote:
> On Sat, Dec 22 2012, Mark Walters  wrote:
>
>> view-mode-enter changed between emacs 23 and emacs 24: the current
>> code makes the error buffer disappear in emacs 24 on quitting it (ie
>> pressing q) but this just kills the buffer without closing the split
>> window in emacs 23.
>>
>> This patch makes the error buffer window disappear in emacs 23
>> too. Since the view-mode-enter function changed we have to test for
>> version and do the correct thing in each case.
>> ---
>>
>> This seems to work but I have only tested on 23.4 and 24.2
>
> I run emacs 23.1.1 to get the documentation of view-mode-enter
> there. So, this patch instructs to delete WINDOW when exiting
> view mode...
>
> Documentation of pop-to-buffer says:
>
> "Select buffer BUFFER-OR-NAME in some window, preferably a different one."
>
> What if pop-up-windows's value is nil -- the content of current window
> is replaced with this view stuff -- and when exiting view mode, the
> window will be deleted ? What happens with emacs 24 in this case ?

Hi 

You are quite right there are problems here under emacs 23: if you
already have a split window when the error occurs in one part the error
is displayed in the other window and then on exit that (previously
existing) window is closed.

What do people think should happen on an error? I, personally, don't
like taking over an existing window, and Jamie liked some of the errors
(eg non-fatal `locked database' tagging errors) to be shown in the
mini-buffer.

I also think it is going to be difficult to get this right: emacs 23 and
24 are different and there are also some user configuration variable
that affect what happens.

Best wishes

Mark



>
> Tomi
>
>> Best wishes
>>
>> Mark
>>
>>
>>
>>  emacs/notmuch-lib.el |8 +---
>>  1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
>> index 77a591d..0407f8a 100644
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -324,15 +324,17 @@ the user dismisses it."
>>  
>>(let ((buf (get-buffer-create "*Notmuch errors*")))
>>  (with-current-buffer buf
>> -  (view-mode-enter nil #'kill-buffer)
>> +  (pop-to-buffer buf)
>> +  (view-mode-enter (when (< emacs-major-version 24)
>> +   (cons (selected-window) (cons nil t)))
>> +   #'kill-buffer)
>>(let ((inhibit-read-only t))
>>  (goto-char (point-max))
>>  (unless (bobp)
>>(insert "\n"))
>>  (insert msg)
>>  (unless (bolp)
>> -  (insert "\n"
>> -(pop-to-buffer buf)))
>> +  (insert "\n"))
>>  
>>  (defun notmuch-check-async-exit-status (proc msg)
>>"If PROC exited abnormally, pop up an error buffer and signal an error.
>> -- 
>> 1.7.9.1
>>
>> ___
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

2012-12-26 Thread David Bremner
Michael Forney  writes:

> On Thu, 18 Oct 2012 00:52:20 +0300, Adrien Bustany  
> wrote:
>> The code of the patches in unchanged, but the formatting issues are now
>> hopefully fixed.
>
> I would like to bump this patch set. I also need these features from
> libnotmuch. Currently there is no way to recover from Xapian errors,
> which is pretty limiting.
>
> If it is the flush/commit that is the issue, I would be happy to make an
> updated patch set.

Hi Michael;

Right, we should use the current name of 'commit'. We also need some way
to check the xapian version in the configure script, and to make sure
the right name is supported.

Jani refers to 

 id:1350487737-32058-2-git-send-email-bgamari.foss at gmail.com

farther up the thread as having some relevant code/ideas.

I think it's reasonable to require Xapian version at least 1.2.0, so
that might simplify the configuration check. 

The other thing I wondered is if the error handling in these routines is
as good as it could be. I guess it isn't worse than what is there, but
especially since you are explicitly wanting to handle error conditions,
it would be good to think if we can do something better than adding yet
another printf to the library.

d