From: Stefan Beller <sbeller@google.com>
Date: Tue, 29 May 2018 17:41:28 GMT
Subject: Re: [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing
Message-ID: <CAGZ79kb7K_uZfUdjsnm065h3EYn=GE3Rma5+yACERFhBBcOL+Q@mail.gmail.com>
In-Reply-To: <20180526135531.4516-2-pclouds@gmail.com>

On Sat, May 26, 2018 at 6:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Instead of hard coding the name-to-id mapping in C code, keep it in an
> array and use a common function to do the parsing. This reduces code
> and also allows us to list all possible color slots later.
>
> This starts using C99 designated initializers more for convenience
> (the first designated initializers have been introduced in builtin/clean.c
> for some time without complaints)

s/for some time without complaints/
in 512f41cfac5 (clean.c: use designated initializer, 2017-07-14) which
is almost a year without complaint/ maybe?

(just in case a resend is needed; please be precise and mention that
other commit so it's easy to see the reasoning there)

> +
> +int lookup_config(const char **mapping, int nr_mapping, const char *var)
> +{
> +       int i;
> +
> +       for (i = 0; i < nr_mapping; i++) {
> +               const char *name = mapping[i];
> +

    if (!name)
        break;

maybe? We do we need to check for 'name' in the next
condition? What input do we expect/allow?

> +               if (name && !strcasecmp(var, name))
> +                       return i;


> +#define LOOKUP_CONFIG(mapping, var) \
> +       lookup_config(mapping, ARRAY_SIZE(mapping), var)
> +int lookup_config(const char **mapping, int nr_mapping, const char *var);

Can you add a comment here saying what mapping is or should look like?