Re: [PATCH v2 2/2] attr: more matching optimizations from .gitignore

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Oct 11, 2012, 01:36 UTC
Message-ID: <CACsJy8CdCN0hBU2WwffNvHOwcC87v2OVJj=SYH4BYYveBpFiQw@mail.gmail.com>
In-Reply-To: <7v626iatt9.fsf@alter.siamese.dyndns.org>

On Thu, Oct 11, 2012 at 4:41 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>>    @@ -690,16 +689,18 @@ static int path_matches(const char *pathname, int pathlen,
>>        * contain the trailing slash
>>        */
>>
>>    -  if (pathlen < baselen ||
>>    +  if (pathlen < baselen + 1 ||
>>           (baselen && pathname[baselen] != '/') ||
>>    -      strncmp(pathname, base, baselen))
>>    +      strncmp_icase(pathname, base, baselen))
>
> Shouldn't the last comparison be
>
>         strncmp_icase(pathname, base, baselen + 1)
>
> instead,

"base" does not contain the trailing slash, so it can only match up to base[baselen-1], then fail at base[baselen], which is '\0'. The "no trailing slash" business in this function is tricky :(

Show 7 quoted lines

> if you are trying to match this part from dir.c where
> baselen does count the trailing slash?
>
>                 if (pathlen < x->baselen ||
>                     (x->baselen && pathname[x->baselen-1] != '/') ||
>                     strncmp_icase(pathname, x->base, x->baselen))
>                         continue;

strncmp_icase() here just needs to compare x->baselen-1 chars (i.e. no trailing slash) as the trailing slash is explicitly checked just above strncmp_icase. But it does not hurt to compare an extra character so I leave it unchanged. But obviously it causes confusion when we try to match this function and the one in attr.c

-- 
Duy