Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD

[Christian Couder <christian.couder@gmail.com>]

On Tue, Mar 28, 2017 at 11:24 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Mar 28, 2017 at 2:15 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:
>>> FreeBSD implements getcwd(3) as a syscall, but falls back to a version
>>> based on readdir(3) if it fails for some reason.  The latter requires
>>> permissions to read and execute path components, while the former does
>>> not.  That means that if our buffer is too small and we're missing
>>> rights we could get EACCES, but we may succeed with a bigger buffer.
>>>
>>> Keep retrying if getcwd(3) indicates lack of permissions until our
>>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the
>>> syscall on FreeBSD anyway.  This way we do what we can to be able to
>>> benefit from the syscall, but we also won't loop forever if there is a
>>> real permission issue.
>>
>> Sorry to be late and maybe I missed something obvious, but the above
>> and the patch seem complex to me compared with something like:
>>
>> diff --git a/strbuf.c b/strbuf.c
>> index ace58e7367..25eadcbedc 100644
>> --- a/strbuf.c
>> +++ b/strbuf.c
>> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char
>> *path, size_t hint)
>>  int strbuf_getcwd(struct strbuf *sb)
>>  {
>>         size_t oldalloc = sb->alloc;
>> -       size_t guessed_len = 128;
>> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;
>>
>>         for (;; guessed_len *= 2) {
>>                 strbuf_grow(sb, guessed_len);
>
> From f22a76e911 (strbuf: add strbuf_getcwd(), 2014-07-28):
>     Because it doesn't use a fixed-size buffer it supports
>     arbitrarily long paths, provided the platform's getcwd() does as well.
>     At least on Linux and FreeBSD it handles paths longer than PATH_MAX
>     just fine.

Well René's patch says:

>>> Keep retrying if getcwd(3) indicates lack of permissions until our
>>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the
>>> syscall on FreeBSD anyway.

So it says that FreeBSD syscall doesn't handle paths longer than PATH_MAX.

> So with your patch, we'd still see the original issue for paths > PATH_MAX
> IIUC.

Also, René's patch just adds:

+               if (errno == EACCES && guessed_len < PATH_MAX)
+                       continue;

so if the length of the path is > PATH_MAX, then guessed_len will have
to be > PATH_MAX and then René's patch will do nothing.