v4 of Hex Dump/Restore patches

Mark Walters markwalters1009 at gmail.com
Sat Dec 8 11:47:16 PST 2012


Aside from the minor nit with one comment this series looks good to
me. (I haven't thoroughly checked patch 8)

Best wishes

Mark

On Sat, 08 Dec 2012, david at tethera.net wrote:
> This obsoletes the series at
>      
>      id:1354843607-17980-1-git-send-email-david at tethera.net
>
> One new patch:
>
> [Patch v4 08/10] test/dump-restore: add test for warning/error
>
> Other changes since the last series are as follows:
>
> commit 16ca0f8126fafd49266ffa02534f2e9b73c03027
> Author: David Bremner <bremner at debian.org>
> Date:   Fri Dec 7 20:19:21 2012 -0400
>
>     fixup for test/dump-restore
>
> diff --git a/test/dump-restore b/test/dump-restore
> index 2532cbb..b267792 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -212,7 +212,7 @@ test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest 'roundtripping random message-ids and tags'
>  
> -    ${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG}
> +    ${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} \
>  			--num-messages=100
>  
>       notmuch dump --format=batch-tag| \
>
> commit 9d864bd724bd2d2b26b5d52e3e2f28b4fff2046f
> Author: David Bremner <bremner at debian.org>
> Date:   Fri Dec 7 20:32:50 2012 -0400
>
>     fixup for tag-util.c: put * in right palce
>
> diff --git a/tag-util.h b/tag-util.h
> index df05d72..6674674 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -62,8 +62,8 @@ parse_tag_line (void *ctx, char *line,
>   * ctx is passed to talloc
>   */
>  
> -tag_op_list_t
> -*tag_op_list_create (void *ctx);
> +tag_op_list_t *
> +tag_op_list_create (void *ctx);
>  
>  /*
>   * Add a tag operation (delete iff remove == TRUE) to a list.
>
> commit d2cead797981e8992849e1d96ad95177b42290e2
> Author: David Bremner <bremner at debian.org>
> Date:   Fri Dec 7 20:57:52 2012 -0400
>
>     fixup for id:87txrxxyzp.fsf at nikula.org
>
> diff --git a/tag-util.c b/tag-util.c
> index 3d54e9e..a927363 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -24,9 +24,15 @@ parse_tag_line (void *ctx, char *line,
>  {
>      char *tok = line;
>      size_t tok_len = 0;
> -    char *line_for_error = talloc_strdup (ctx, line);
> +    char *line_for_error;
>      int ret = 0;
>  
> +    line_for_error = talloc_strdup (ctx, line);
> +    if (line_for_error == NULL) {
> +	fprintf (stderr, "Error: out of memory\n");
> +	return -1;
> +    }
> +
>      chomp_newline (line);
>  
>      /* remove leading space */
> @@ -58,7 +64,7 @@ parse_tag_line (void *ctx, char *line,
>  
>  	/* If tag is terminated by NUL, there's no query string. */
>  	if (*(tok + tok_len) == '\0') {
> -	    fprintf (stderr, "no query string: %s\n", line_for_error);
> +	    fprintf (stderr, "Warning: no query string: %s\n", line_for_error);
>  	    ret = 1;
>  	    goto DONE;
>  	}
> @@ -71,19 +77,21 @@ parse_tag_line (void *ctx, char *line,
>  
>  	/* Maybe refuse empty tags. */
>  	if (! (flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') {
> -	    fprintf (stderr, "Error: empty tag: %s\n", line_for_error);
> +	    fprintf (stderr, "Warning: empty tag: %s\n", line_for_error);
> +	    ret = 1;
>  	    goto DONE;
>  	}
>  
>  	/* Decode tag. */
>  	if (hex_decode_inplace (tag) != HEX_SUCCESS) {
> -	    fprintf (stderr, "Hex decoding of tag %s failed\n",
> +	    fprintf (stderr, "Warning: Hex decoding of tag %s failed\n",
>  		     tag);
>  	    ret = 1;
>  	    goto DONE;
>  	}
>  
>  	if (tag_op_list_append (ctx, tag_ops, tag, remove)) {
> +	    /* diagnostics already printed */
>  	    ret = -1;
>  	    goto DONE;
>  	}
> @@ -98,7 +106,7 @@ parse_tag_line (void *ctx, char *line,
>  
>      /* tok now points to the query string */
>      if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> -	fprintf (stderr, "Hex decoding of query %s failed\n",
> +	fprintf (stderr, "Warning: Hex decoding of query %s failed\n",
>  		 tok);
>  	ret = 1;
>  	goto DONE;
>
> commit 162c20ab6d9b9eef56cb2b966a7144a61557eade
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 07:29:56 2012 -0400
>
>     fixup for id:87624chmfw.fsf at qmul.ac.uk
>     
>     update comment
>
> diff --git a/util/string-util.h b/util/string-util.h
> index 696da40..ac7676c 100644
> --- a/util/string-util.h
> +++ b/util/string-util.h
> @@ -3,7 +3,10 @@
>  
>  #include <string.h>
>  
> -/* like strtok(3), but without state, and doesn't modify s. usage pattern:
> +/* like strtok(3), but without state, and doesn't modify s.  Return
> + * value is indicated by pointer and length, not null terminator.
> + *
> + * Usage pattern:
>   *
>   * const char *tok = input;
>   * const char *delim = " \t";
>
> commit 590adeec072f235861154b930bfb10bedbe37e8a
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 07:42:44 2012 -0400
>
>     fixup for id:8738zghl6d.fsf at qmul.ac.uk
>
> diff --git a/tag-util.c b/tag-util.c
> index a927363..c97d240 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -98,8 +98,6 @@ parse_tag_line (void *ctx, char *line,
>      }
>  
>      if (tok == NULL) {
> -	fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
> -		 line_for_error);
>  	ret = 1;
>  	goto DONE;
>      }
> @@ -113,7 +111,13 @@ parse_tag_line (void *ctx, char *line,
>      }
>  
>      *query_string = tok;
> +
>    DONE:
> +    if (ret != 0)
> +	fprintf (stderr, "% invalid input line %s\n",
> +		 ret == 1 ? "Warning: Ignoring" : "Error: Failing at",
> +		 line_for_error);
> +
>      talloc_free (line_for_error);
>      return ret;
>  }
>
> commit 6002e08eb048612f5ef4eab3205a2887e1ab3f02
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 07:46:39 2012 -0400
>
>     fixup: chomp line before copy
>
> diff --git a/tag-util.c b/tag-util.c
> index c97d240..8e6b1ef 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -27,14 +27,14 @@ parse_tag_line (void *ctx, char *line,
>      char *line_for_error;
>      int ret = 0;
>  
> +    chomp_newline (line);
> +
>      line_for_error = talloc_strdup (ctx, line);
>      if (line_for_error == NULL) {
>  	fprintf (stderr, "Error: out of memory\n");
>  	return -1;
>      }
>  
> -    chomp_newline (line);
> -
>      /* remove leading space */
>      while (*tok == ' ' || *tok == '\t')
>  	tok++;
>
> commit 9c4878bf0ad0e1f60a45c963998a20635fc234af
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 08:36:48 2012 -0400
>
>     further fixup for error messages
>
> diff --git a/tag-util.c b/tag-util.c
> index 8e6b1ef..6018d49 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -41,7 +41,7 @@ parse_tag_line (void *ctx, char *line,
>  
>      /* Skip empty and comment lines. */
>      if (*tok == '\0' || *tok == '#') {
> -	ret = 1;
> +	ret = 2;
>  	goto DONE;
>      }
>  
> @@ -55,6 +55,8 @@ parse_tag_line (void *ctx, char *line,
>  	/* Optional explicit end of tags marker. */
>  	if (tok_len == 2 && strncmp (tok, "--", tok_len) == 0) {
>  	    tok = strtok_len (tok + tok_len, " ", &tok_len);
> +	    if (tok == NULL)
> +		fprintf (stderr, "Warning: no query string: %s\n", line_for_error);
>  	    break;
>  	}
>  
> @@ -113,8 +115,8 @@ parse_tag_line (void *ctx, char *line,
>      *query_string = tok;
>  
>    DONE:
> -    if (ret != 0)
> -	fprintf (stderr, "% invalid input line %s\n",
> +    if ((ret % 2) != 0)
> +	fprintf (stderr, "%s invalid input line %s\n",
>  		 ret == 1 ? "Warning: Ignoring" : "Error: Failing at",
>  		 line_for_error);
>  
> diff --git a/tag-util.h b/tag-util.h
> index 6674674..1d564f0 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -45,7 +45,10 @@ typedef enum {
>   * Leading and trailing space ' ' is ignored. Empty lines and lines
>   * beginning with '#' are ignored.
>   *
> - * Returns: 0 for OK, 1 for skipped line, -1 for fatal(ish) error.
> + * Returns:	0	OK,
> + *		1	skipped (invalid) line
> + *		2	skipped (valid)
> + *		-1	fatal(ish) error.
>   *
>   * Output Parameters:
>   *	ops	contains a list of tag operations
>
> commit 6e8a7b30e3980f77724d6005292265d49ad5241a
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 08:41:01 2012 -0400
>
>     fixup for id:87wqwsg46p.fsf at qmul.ac.uk
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index ceec2d3..44bf88d 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -56,6 +56,11 @@ tag_message (unused (void *ctx),
>      return ret;
>  }
>  
> +/* Sup dump output is one line per message. We match a sequence of
> + * non-space characters for the message-id, then one or more
> + * spaces, then a list of space-separated tags as a sequence of
> + * characters within literal '(' and ')'. */
> +
>  static int
>  parse_sup_line (void *ctx, char *line,
>  		char **query_str, tag_op_list_t *tag_ops)
> @@ -172,11 +177,6 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
>  		 argv[opt_index]);
>  	return 1;
>      }
> -
> -    /* Dump output is one line per message. We match a sequence of
> -     * non-space characters for the message-id, then one or more
> -     * spaces, then a list of space-separated tags as a sequence of
> -     * characters within literal '(' and ')'. */
>      char *p;
>  
>      line_len = getline (&line, &line_size, input);
>
> commit ee198b0d388243f0f43fab1d7ef7acbea5bdb3e6
> Author: David Bremner <bremner at debian.org>
> Date:   Sat Dec 8 08:52:51 2012 -0400
>
>     fixup: rewrite confusing for loop as while.
>
> diff --git a/tag-util.c b/tag-util.c
> index 6018d49..b68ea50 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -157,8 +157,10 @@ makes_changes (notmuch_message_t *message,
>  	const char *cur_tag = notmuch_tags_get (tags);
>  	int last_op =  (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0;
>  
> -	/* slight contortions to count down with an unsigned index */
> -	for (i = list->count; i-- > 0; /*nothing*/) {
> +	/* scan backwards to get last operation */
> +	i = list->count;
> +	while (i > 0) {
> +	    i--;
>  	    if (strcmp (cur_tag, list->ops[i].tag) == 0) {
>  		last_op = list->ops[i].remove ? -1 : 1;
>  		break;
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


More information about the notmuch mailing list