Re: [PATCH 2/2] convert trivial uses of strncmp() to skip_prefix()

From: René Scharfe <l.s.r@web.de>
Date: Jan 7, 2023, 21:29 UTC
Message-ID: <e525342d-5900-1870-f176-da4eef083143@web.de>
In-Reply-To: <Y7l0mq6khtlgxGca@coredump.intra.peff.net>

Am 07.01.23 um 14:33 schrieb Jeff King:

Show 26 quoted lines

> On Sat, Jan 07, 2023 at 08:26:45AM -0500, Jeff King wrote:
>
>> Note in the case in ws.c that to get rid of the magic number "9"
>> completely, we also switch out "len" for recomputing the pointer
>> difference. These are equivalent because "len" is always "ep - string".
>
> By the way, one thing I noticed about this parse_whitespace_rule()
> function is that it's very loose about its matching. It does:
>
>         for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) {
>                 if (strncmp(whitespace_rule_names[i].rule_name,
>                             string, len))
>                         continue;
> 		...we matched...
> 		break;
> 	}
>
> So it will prefix-match any of the options, even if there are
> ambiguities. E.g.:
>
>   git -c core.whitespace=-t show
>
> will turn off "trailing-space", even though it would also match
> "tab-in-indent". It would be easy enough to fix it to require the whole
> name, but I wasn't sure if this prefix-matching was supposed to be a
> feature (it doesn't seem to be documented anywhere, though).

Abbreviations are being used:

   $ git grep whitespace= .gitattributes
   .gitattributes:* whitespace=!indent,trail,space
   .gitattributes:*.[ch] whitespace=indent,trail,space diff=cpp
   .gitattributes:*.sh whitespace=indent,trail,space eol=lf

(Full names: trailing-space, space-before-tab, indent-with-non-tab.)

a9cc857ada (War on whitespace: first, a bit of retreat., 2007-11-02) added this function. Its commit message says:

   "You can specify the desired types of errors to be detected by
    listing their names (unique abbreviations are accepted)
    separated by comma."

And the part about abbreviations needing to be unique was true at that time, because the names of the only two types back then started with different characters. aeb84b05ae (core.whitespace: split trailing-space into blank-at-{eol,eof}, 2009-09-05) broke that property first.

The current behavior of picking the first match from a list with a basically random order is not ideal, but at least stable. Can we still change it? The intent seems to have been to ignore ambiguous prefix matches.

René