Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer

[Stefan Beller <sbeller@google.com>]

On Thu, Oct 20, 2016 at 2:39 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
> The parse_trailer function has a few modes of operation, all depending
> on whether the separator is present in its input, and if yes, the
> separator's position. Some of these modes are failure modes, and these
> failure modes are handled differently depending on whether the trailer
> line was sourced from a file or from a command-line argument.
>
> Extract a function to find the separator, allowing the invokers of
> parse_trailer to determine how to handle the failure modes instead of
> making parse_trailer do it.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  trailer.c | 70 +++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index 99018f8..137a3fb 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -543,29 +543,40 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
>         return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
>  }
>
> -static int parse_trailer(struct strbuf *tok, struct strbuf *val,
> -                        const struct conf_info **conf, const char *trailer)
> +/*
> + * Return the location of the first separator or '=' in line, or -1 if either a
> + * newline or the null terminator is reached first.
> + */
> +static int find_separator(const char *line)
> +{
> +       const char *c;
> +       for (c = line; ; c++) {
> +               if (!*c || *c == '\n')
> +                       return -1;
> +               if (*c == '=' || strchr(separators, *c))
> +                       return c - line;
> +       }

I was about to suggest this function can be simplified and maybe
even inlined by the use of strspn or strcspn, but I think manual
processing of the string is fine, too, as it would not really be shorter.