Subject: Re: [PATCH v4 3/4] setup: Add 'abspath_part_inside_repo' function

From: Torsten Bögershausen <[email-masked]>
Date: Sun, 02 Feb 2014 12:37:16 +0100

On 2014-02-02 12.21, David Kastrup wrote:
> Martin Erik Werner <[email-masked]> writes:
> 
>> On Sun, Feb 02, 2014 at 09:19:04AM +0700, Duy Nguyen wrote:
>>> On Sun, Feb 2, 2014 at 8:59 AM, Martin Erik Werner
>>> <[email-masked]> wrote:
>>>> +       /* check if work tree is already the prefix */
>>>> +       if (strncmp(path, work_tree, wtlen) == 0) {
>>>> +               if (path[wtlen] == '/')
>>>> +                       memmove(path, path + wtlen + 1, len - wtlen);
>>>> +               else
>>>> +                       /* work tree is the root, or the whole path */
>>>> +                       memmove(path, path + wtlen, len - wtlen + 1);
>>>> +               return 0;
>>>> +       }
>>>
>>> No the 4th time is not the charm yet :) if path is "/abc/defghi" and
>>> work_tree is "/abc/def" you don't want to return "ghi" as the prefix
>>> here.
>>
>> Ah indeed, this should catch that:
>>
>> diff --git a/setup.c b/setup.c
>> index 2270bd4..5817875 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -32,9 +32,11 @@ static inline int abspath_part_inside_repo(char *path)
>>  	if (strncmp(path, work_tree, wtlen) == 0) {
>>  		if (path[wtlen] == '/')
>>  			memmove(path, path + wtlen + 1, len - wtlen);
>> -		else
>> +		else if (path[wtlen - 1] == '/' || path[wtlen] == '\0')
> 
> Is wtlen guaranteed to be nonzero?
> 
Another comment:
The "raw" comparison with '/' is probably working well on all
POSIX/Linux/Unix systems.

To be more portable, the macro
is_dir_sep()
can be used:

if (is_dir_sep(path[wtlen]))