Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD
- From
Stefan Beller <sbeller@google.com>
- Date
- Mar 28, 2017, 21:24 UTC
- Message-ID
- <CAGZ79kaFhrEFFfmBUUM7rc9RftS3cKLcvJe5M8jkwKL=bdemgQ@mail.gmail.com>
- In-Reply-To
- <CAP8UFD3pXSf+RhysULQyd2kdKSkBWdoKs+L6GPQ4jwpoeP+yOA@mail.gmail.com>
On Tue, Mar 28, 2017 at 2:15 PM, Christian Couder <christian.couder@gmail.com> wrote:
Show 30 quoted lines
> 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.So with your patch, we'd still see the original issue for paths > PATH_MAX IIUC.
Thanks, Stefan