[Patch v4 04/10] tag-util.[ch]: New files for common tagging routines
Jani Nikula
jani at nikula.org
Sat Dec 8 11:22:22 PST 2012
Hi David, some error message bikeshedding below.
BR,
Jani.
On Sat, 08 Dec 2012, david at tethera.net wrote:
> From: David Bremner <bremner at debian.org>
>
> These are meant to be shared between notmuch-tag and notmuch-restore.
>
> The bulk of the routines implement a "tag operation list" abstract
> data type act as a structured representation of a set of tag
> operations (typically coming from a single tag command or line of
> input).
> ---
> Makefile.local | 1 +
> tag-util.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tag-util.h | 122 +++++++++++++++++++++++
> 3 files changed, 415 insertions(+)
> create mode 100644 tag-util.c
> create mode 100644 tag-util.h
>
> diff --git a/Makefile.local b/Makefile.local
> index 0db1713..c274f07 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -275,6 +275,7 @@ notmuch_client_srcs = \
> query-string.c \
> mime-node.c \
> crypto.c \
> + tag-util.c
>
> notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
>
> diff --git a/tag-util.c b/tag-util.c
> new file mode 100644
> index 0000000..80ebdc2
> --- /dev/null
> +++ b/tag-util.c
> @@ -0,0 +1,292 @@
> +#include <assert.h>
> +#include "string-util.h"
> +#include "tag-util.h"
> +#include "hex-escape.h"
> +
> +#define TAG_OP_LIST_INITIAL_SIZE 10
> +
> +struct _tag_operation_t {
> + const char *tag;
> + notmuch_bool_t remove;
> +};
> +
> +struct _tag_op_list_t {
> + tag_operation_t *ops;
> + size_t count;
> + size_t size;
> +};
> +
> +int
> +parse_tag_line (void *ctx, char *line,
> + tag_op_flag_t flags,
> + char **query_string,
> + tag_op_list_t *tag_ops)
> +{
> + char *tok = line;
> + size_t tok_len = 0;
> + 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;
> + }
> +
> + /* remove leading space */
> + while (*tok == ' ' || *tok == '\t')
> + tok++;
> +
> + /* Skip empty and comment lines. */
> + if (*tok == '\0' || *tok == '#') {
> + ret = 2;
> + goto DONE;
> + }
> +
> + tag_op_list_reset (tag_ops);
> +
> + /* Parse tags. */
> + while ((tok = strtok_len (tok + tok_len, " ", &tok_len)) != NULL) {
> + notmuch_bool_t remove;
> + char *tag;
> +
> + /* 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);
I think you should move this warning back... (see below)
> + break;
> + }
> +
> + /* Implicit end of tags. */
> + if (*tok != '-' && *tok != '+')
> + break;
> +
> + /* If tag is terminated by NUL, there's no query string. */
> + if (*(tok + tok_len) == '\0') {
> + fprintf (stderr, "Warning: no query string: %s\n", line_for_error);
> + ret = 1;
> + goto DONE;
> + }
> +
> + /* Terminate, and start next token after terminator. */
> + *(tok + tok_len++) = '\0';
> +
> + remove = (*tok == '-');
> + tag = tok + 1;
> +
> + /* Maybe refuse empty tags. */
> + if (! (flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') {
> + 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, "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;
> + }
> + }
> +
> + if (tok == NULL) {
...here where it was. Now you'll only get the warning for lines like:
+foo +bar --
but not for:
+foo +bar
which are only caught here. (Alternatively, you could have both print
the error message, and set ret and goto DONE also in the first case. But
I don't know if that gains us anything.)
> + ret = 1;
> + goto DONE;
> + }
> +
> + /* tok now points to the query string */
> + if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + fprintf (stderr, "Warning: Hex decoding of query %s failed\n",
> + tok);
> + ret = 1;
> + goto DONE;
> + }
> +
> + *query_string = tok;
> +
> + DONE:
> + if ((ret % 2) != 0)
> + fprintf (stderr, "%s invalid input line %s\n",
> + ret == 1 ? "Warning: Ignoring" : "Error: Failing at",
> + line_for_error);
Can't resist urge to nitpick... I think the (ret % 2) is a bit
magical. I'd prefer the explicit (ret != 0 && ret != 2). Another one is
that some of the earlier warnings/errors already print line_for_error,
resulting in printing the line twice. Maybe only print line_for_error
here, and remove that bit from the messages earlier?
> +
> + talloc_free (line_for_error);
> + return ret;
> +}
> +
> +static inline void
> +message_error (notmuch_message_t *message,
> + notmuch_status_t status,
> + const char *format, ...)
> +{
> + va_list va_args;
> +
> + va_start (va_args, format);
> +
> + vfprintf (stderr, format, va_args);
> + fprintf (stderr, "Message-ID: %s\n", notmuch_message_get_message_id (message));
> + fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));
> +}
> +
> +notmuch_status_t
> +tag_op_list_apply (notmuch_message_t *message,
> + tag_op_list_t *list,
> + tag_op_flag_t flags)
> +{
> + size_t i;
> + notmuch_status_t status = 0;
> + tag_operation_t *tag_ops = list->ops;
> +
> + status = notmuch_message_freeze (message);
> + if (status) {
> + message_error (message, status, "freezing message");
> + return status;
> + }
> +
> + if (flags & TAG_FLAG_REMOVE_ALL) {
> + status = notmuch_message_remove_all_tags (message);
> + if (status) {
> + message_error (message, status, "removing all tags");
> + return status;
> + }
> + }
> +
> + for (i = 0; i < list->count; i++) {
> + if (tag_ops[i].remove) {
> + status = notmuch_message_remove_tag (message, tag_ops[i].tag);
> + if (status) {
> + message_error (message, status, "removing tag %s", tag_ops[i].tag);
> + return status;
> + }
> + } else {
> + status = notmuch_message_add_tag (message, tag_ops[i].tag);
> + if (status) {
> + message_error (message, status, "adding tag %s", tag_ops[i].tag);
> + return status;
> + }
> +
> + }
> + }
> +
> + status = notmuch_message_thaw (message);
> + if (status) {
> + message_error (message, status, "thawing message");
> + return status;
> + }
> +
> +
> + if (flags & TAG_FLAG_MAILDIR_SYNC) {
> + status = notmuch_message_tags_to_maildir_flags (message);
> + if (status) {
> + message_error (message, status, "synching tags to maildir");
> + return status;
> + }
> + }
> +
> + return NOTMUCH_STATUS_SUCCESS;
> +
> +}
> +
> +
> +/* Array of tagging operations (add or remove. Size will be increased
> + * as necessary. */
> +
> +tag_op_list_t *
> +tag_op_list_create (void *ctx)
> +{
> + tag_op_list_t *list;
> +
> + list = talloc (ctx, tag_op_list_t);
> + if (list == NULL)
> + return NULL;
> +
> + list->size = TAG_OP_LIST_INITIAL_SIZE;
> + list->count = 0;
> +
> + list->ops = talloc_array (ctx, tag_operation_t, list->size);
> + if (list->ops == NULL)
> + return NULL;
> +
> + return list;
> +}
> +
> +
> +int
> +tag_op_list_append (void *ctx,
> + tag_op_list_t *list,
> + const char *tag,
> + notmuch_bool_t remove)
> +{
> + /* Make room if current array is full. This should be a fairly
> + * rare case, considering the initial array size.
> + */
> +
> + if (list->count == list->size) {
> + list->size *= 2;
> + list->ops = talloc_realloc (ctx, list->ops, tag_operation_t,
> + list->size);
> + if (list->ops == NULL) {
> + fprintf (stderr, "Out of memory.\n");
> + return 1;
> + }
> + }
> +
> + /* add the new operation */
> +
> + list->ops[list->count].tag = tag;
> + list->ops[list->count].remove = remove;
> + list->count++;
> + return 0;
> +}
> +
> +/*
> + * Is the i'th tag operation a remove?
> + */
> +
> +notmuch_bool_t
> +tag_op_list_isremove (const tag_op_list_t *list, size_t i)
> +{
> + assert (i < list->count);
> + return list->ops[i].remove;
> +}
> +
> +/*
> + * Reset a list to contain no operations
> + */
> +
> +void
> +tag_op_list_reset (tag_op_list_t *list)
> +{
> + list->count = 0;
> +}
> +
> +/*
> + * Return the number of operations in a list
> + */
> +
> +size_t
> +tag_op_list_size (const tag_op_list_t *list)
> +{
> + return list->count;
> +}
> +
> +/*
> + * return the i'th tag in the list
> + */
> +
> +const char *
> +tag_op_list_tag (const tag_op_list_t *list, size_t i)
> +{
> + assert (i < list->count);
> + return list->ops[i].tag;
> +}
> diff --git a/tag-util.h b/tag-util.h
> new file mode 100644
> index 0000000..581207a
> --- /dev/null
> +++ b/tag-util.h
> @@ -0,0 +1,122 @@
> +#ifndef _TAG_UTIL_H
> +#define _TAG_UTIL_H
> +
> +#include "notmuch-client.h"
> +
> +typedef struct _tag_operation_t tag_operation_t;
> +typedef struct _tag_op_list_t tag_op_list_t;
> +
> +/* Use powers of 2 */
> +typedef enum {
> + TAG_FLAG_NONE = 0,
> +
> + /* Operations are synced to maildir, if possible.
> + */
> + TAG_FLAG_MAILDIR_SYNC = (1 << 0),
> +
> + /* Remove all tags from message before applying list.
> + */
> + TAG_FLAG_REMOVE_ALL = (1 << 1),
> +
> + /* Don't try to avoid database operations. Useful when we
> + * know that message passed needs these operations.
> + */
> + TAG_FLAG_PRE_OPTIMIZED = (1 << 2),
> +
> + /* Accept strange tags that might be user error;
> + * intended for use by notmuch-restore.
> + */
> + TAG_FLAG_BE_GENEROUS = (1 << 3)
> +
> +} tag_op_flag_t;
> +
> +/* Parse a string of the following format:
> + *
> + * +<tag>|-<tag> [...] [--] <search-terms>
> + *
> + * Each line is interpreted similarly to "notmuch tag" command line
> + * arguments. The delimiter is one or more spaces ' '. Any characters
> + * in <tag> and <search-terms> MAY be hex encoded with %NN where NN is
> + * the hexadecimal value of the character. Any ' ' and '%' characters
> + * in <tag> and <search-terms> MUST be hex encoded (using %20 and %25,
> + * respectively). Any characters that are not part of <tag> or
> + * <search-terms> MUST NOT be hex encoded.
> + *
> + * Leading and trailing space ' ' is ignored. Empty lines and lines
> + * beginning with '#' are ignored.
> + *
> + * Returns: 0 OK,
> + * 1 skipped (invalid) line
> + * 2 skipped (valid) line
> + * -1 fatal(ish) error.
> + *
> + * Output Parameters:
> + * ops contains a list of tag operations
> + * query_str the search terms.
> + */
> +int
> +parse_tag_line (void *ctx, char *line,
> + tag_op_flag_t flags,
> + char **query_str, tag_op_list_t *ops);
> +
> +/*
> + * Create an empty list of tag operations
> + *
> + * ctx is passed to talloc
> + */
> +
> +tag_op_list_t *
> +tag_op_list_create (void *ctx);
> +
> +/*
> + * Add a tag operation (delete iff remove == TRUE) to a list.
> + * The list is expanded as necessary.
> + */
> +
> +int
> +tag_op_list_append (void *ctx,
> + tag_op_list_t *list,
> + const char *tag,
> + notmuch_bool_t remove);
> +
> +/*
> + * Apply a list of tag operations, in order, to a given message.
> + *
> + * Flags can be bitwise ORed; see enum above for possibilies.
> + */
> +
> +notmuch_status_t
> +tag_op_list_apply (notmuch_message_t *message,
> + tag_op_list_t *tag_ops,
> + tag_op_flag_t flags);
> +
> +/*
> + * Return the number of operations in a list
> + */
> +
> +size_t
> +tag_op_list_size (const tag_op_list_t *list);
> +
> +/*
> + * Reset a list to contain no operations
> + */
> +
> +void
> +tag_op_list_reset (tag_op_list_t *list);
> +
> +
> + /*
> + * return the i'th tag in the list
> + */
> +
> +const char *
> +tag_op_list_tag (const tag_op_list_t *list, size_t i);
> +
> +/*
> + * Is the i'th tag operation a remove?
> + */
> +
> +notmuch_bool_t
> +tag_op_list_isremove (const tag_op_list_t *list, size_t i);
> +
> +#endif
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list