Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()

From: René Scharfe <l.s.r@web.de>
Date: Jul 20, 2014, 11:17 UTC
Message-ID: <53CBA55B.5010606@web.de>
In-Reply-To: <53CB0575.5020707@gmail.com>

Am 20.07.2014 01:55, schrieb Karsten Blees:

Show 26 quoted lines

> Am 18.07.2014 13:32, schrieb René Scharfe:
>> Am 18.07.2014 01:03, schrieb Karsten Blees:
>>> Am 17.07.2014 19:05, schrieb René Scharfe:
>>>> Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy:
>>> [...]
>>>> "These routines have traditionally been used by programs to
>>>> save the name of a working directory for the purpose of
>>>> returning to it. A much faster and less error-prone method of
>>>> accomplishing this is to open the current directory (.) and use
>>>> the fchdir(2) function to return."
>>>>
>>>
>>> fchdir() is part of the POSIX-XSI extension, as is realpath(). So
>>> why not use realpath() directly (which would also be
>>> thread-safe)?
>>
>> That's a good question; thanks for stepping back and looking at the
>> bigger picture.  If there is widespread OS support for a
>> functionality then we should use it and just provide a
>> compatibility implementation for those platforms lacking it.  The
>> downside is that compat code gets less testing.
>>
>
> I just noticed that in contrast to the POSIX realpath(), our
> real_path() doesn't require the last path component to exist. I don't
> know if this property is required by the calling code, though.

If it's important then removing the last component on ENOENT, calling realpath() again and appending it should handle that, right?

Show 12 quoted lines

>> Seeing that readlink()
>
> You mean realpath()? We don't have a stub for that yet.
>
>> is left as a stub in compat/mingw.h that only errors out, would the
>> equivalent function on Windows be PathCanonicalize
>> (http://msdn.microsoft.com/en-us/library/windows/desktop/bb773569%28v=vs.85%29.aspx)?
>>
> PathCanonicalize() doesn't return an absolute path, the realpath()
> equivalent would be GetFullPathName() (doesn't resolve symlinks) or
> GetFinalPathNameByHandle() (requires Vista, resolves symlinks,
> requires the path to exist).

I meant readlink() and assumed its ENOSYS stub in compat/mingw.h means that git doesn't handle symlinks on Windows at all.

> However, a POSIX conformant getcwd must fail with ERANGE if the
> buffer is too small. So a better alternative would be to add a
> strbuf_getcwd() that works similar to strbuf_readlink() (i.e. resize
> the buffer until its large enough).

That's a good idea anyway, will send patches.

> Side note: the 'hard' 260 limit for the current directory also means
> that as long as we *simulate* realpath() via chdir()/getcwd(), long
> paths [1] don't work here.

Great, so that motivation for getting a chdir()-free implementation remains. :)