[Patch v6 1/6] tag-util: factor out rules for illegal tags, use in parse_tag_line
Jani Nikula
jani at nikula.org
Mon Dec 10 12:55:38 PST 2012
On Mon, 10 Dec 2012, david at tethera.net wrote:
> From: David Bremner <bremner at debian.org>
>
> This will allow us to be consistent between batch tagging and command
> line tagging as far as what is an illegal tag.
LGTM, with some nitpick and observations below.
Jani.
> ---
> tag-util.c | 35 ++++++++++++++++++++++++++++++-----
> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/tag-util.c b/tag-util.c
> index eab482f..c071ea8 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -31,6 +31,29 @@ line_error (tag_parse_status_t status,
> return status;
> }
>
> +/*
> + * Test tags for some forbidden cases.
> + *
> + * return: NULL if OK,
> + * explanatory message otherwise.
> + */
> +
> +static const char *
> +illegal_tag (const char *tag, notmuch_bool_t remove) {
Add \n before opening brace.
> +
> + if (*tag == '\0' && !remove)
> + return "adding empty tag";
> +
> + /* This disallows adding the non-removable tag "-" and
> + * enables notmuch tag to take long options more easily.
> + */
Maybe make that: "This disallows adding tags starting with "-",
including the non-removable tag "-", and enables ...", or similar?
> +
> + if (*tag == '-' && !remove)
> + return "adding tag starting with -";
> +
> + return NULL;
> +}
> +
> tag_parse_status_t
> parse_tag_line (void *ctx, char *line,
> tag_op_flag_t flags,
> @@ -95,11 +118,13 @@ parse_tag_line (void *ctx, char *line,
> remove = (*tok == '-');
> tag = tok + 1;
>
> - /* Maybe refuse empty tags. */
> - if (! (flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') {
> - ret = line_error (TAG_PARSE_INVALID, line_for_error,
> - "empty tag");
> - goto DONE;
> + /* Maybe refuse illegal tags. */
> + if (! (flags & TAG_FLAG_BE_GENEROUS)) {
> + const char *msg = illegal_tag (tag, remove);
> + if (msg) {
> + ret = line_error (TAG_PARSE_INVALID, line_for_error, msg);
> + goto DONE;
> + }
> }
I guess I failed to notice during the dump/restore review that this
function was (and still is until later in the series) always called with
TAG_FLAG_BE_GENEROUS. I guess that's what we want with restore;
otherwise we should warn about such tags during dump.
>
> /* Decode tag. */
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list