Re: [PATCH v2 6/7] gc: don't run "reflog expire" when keeping reflogs

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>>> +       if (gc_config_is_timestamp_never("gc.reflogexpire") &&
>>> +           gc_config_is_timestamp_never("gc.reflogexpireunreachable"))
>>
>> Nit. configset api normalizes the key internally, so we can safely
>> write gc.reflogExpireUnreachable here, which is a bit easier to read.
>
> I didn't know that, do we want to do that?
>
> I'd think that as a matter of coding style always sticking to lower case
> in the C code made more sense, because e.g. you might #define a config
> key, and then use it both in git_config_*() as well as via strcmp() in
> some callback. Mixing the two would lead to confusion and possible bugs
> as we'd refactor the former of those patterns to the latter.

When parsing end-user supplied value in a "const char *var", with

    #define REU "gc.reflogexpireunreacahble"

strcmp(var, REU) is wrong.  You must use strcasecmp(var, REU)
because the end-user supplied value may be in camelCase.

And once the code uses strcasecmp(var, REU), using camelCase

    #define REU "gc.reflogExpireUnreacahble"

to compare against would not break it, either.  The only difference
is that being able to see the word boundary may make it easier to
read.

So even when git_config-*() and str*cmp() are used in a mixed way, I
do not think it is a good justification to make the string constant
all lowercase (assuming that camel is easier than all lowercase to
read, that is).

Parsing three-level names is tricky.  Without breaking it down into
components first, you cannot use strcmp nor strcasecmp; comparing
the whole variable name with either function is wrong.