{"thread":{"id":"45517","subject":"[PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","startedAt":"2017-03-26T13:50:56Z","lastAt":"2017-03-30T18:02:24Z","messageCount":10,"participants":["René Scharfe","Junio C Hamano","Zenobiusz Kunegunda","Christian Couder","Stefan Beller","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"315461","messageId":"4026bc3b-2999-9daf-d6ab-10c6d007b1e7@web.de","threadId":"45517","inReplyTo":null,"subject":"[PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-03-26T13:43:50Z","receivedAt":"2017-03-26T13:50:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"FreeBSD implements getcwd(3) as a syscall, but falls back to a version\nbased on readdir(3) if it fails for some reason.  The latter requires\npermissions to read and execute path components, while the former does\nnot.  That means that if our buffer is too small and we're missing\nrights we could get EACCES, but we may succeed with a bigger buffer.\n\nKeep retrying if getcwd(3) indicates lack of permissions until our\nbuffer can fit PATH_MAX bytes, as that's the maximum supported by the\nsyscall on FreeBSD anyway.  This way we do what we can to be able to\nbenefit from the syscall, but we also won't loop forever if there is a\nreal permission issue.\n\nThis fixes a regression introduced with 7333ed17 (setup: convert\nsetup_git_directory_gently_1 et al. to strbuf, 2014-07-28) for paths\nlonger than 127 bytes with components that miss read or execute\npermissions (e.g. 0711 on /home for privacy reasons); we used a fixed\nPATH_MAX-sized buffer before.\n\nReported-by: Zenobiusz Kunegunda <zenobiusz.kunegunda@interia.pl>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n strbuf.c        | 11 +++++++++++\n t/t0001-init.sh | 14 ++++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex ace58e7367..00457940cf 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -449,6 +449,17 @@ int strbuf_getcwd(struct strbuf *sb)\n \t\t\tstrbuf_setlen(sb, strlen(sb->buf));\n \t\t\treturn 0;\n \t\t}\n+\n+\t\t/*\n+\t\t * If getcwd(3) is implemented as a syscall that falls\n+\t\t * back to a regular lookup using readdir(3) etc. then\n+\t\t * we may be able to avoid EACCES by providing enough\n+\t\t * space to the syscall as it's not necessarily bound\n+\t\t * to the same restrictions as the fallback.\n+\t\t */\n+\t\tif (errno == EACCES && guessed_len < PATH_MAX)\n+\t\t\tcontinue;\n+\n \t\tif (errno != ERANGE)\n \t\t\tbreak;\n \t}\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex e424de5363..5f81fbe07c 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -315,6 +315,20 @@ test_expect_success 'init with separate gitdir' '\n \ttest_path_is_dir realgitdir/refs\n '\n \n+test_expect_success 'init in long base path' '\n+\t# exceed initial buffer size of strbuf_getcwd()\n+\tcomponent=123456789abcdef &&\n+\ttest_when_finished \"chmod 0700 $component; rm -rf $component\" &&\n+\tp31=$component/$component &&\n+\tp127=$p31/$p31/$p31/$p31 &&\n+\tmkdir -p $p127 &&\n+\tchmod 0111 $component &&\n+\t(\n+\t\tcd $p127 &&\n+\t\tgit init newdir\n+\t)\n+'\n+\n test_expect_success 're-init on .git file' '\n \t( cd newdir && git init )\n '\n-- \n2.12.2\n\n"},{"id":"315493","messageId":"xmqq1stj4kmp.fsf@gitster.mtv.corp.google.com","threadId":"45517","inReplyTo":"4026bc3b-2999-9daf-d6ab-10c6d007b1e7@web.de","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-27T00:40:46Z","receivedAt":"2017-03-27T00:42:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n> based on readdir(3) if it fails for some reason.  The latter requires\n> permissions to read and execute path components, while the former does\n> not.  That means that if our buffer is too small and we're missing\n> rights we could get EACCES, but we may succeed with a bigger buffer.\n\nWOW.  Just WOW.  Looking at the debugging exchange from the\nsideline, I didn't expect this end result.\n\n> Keep retrying if getcwd(3) indicates lack of permissions until our\n> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n> syscall on FreeBSD anyway.  This way we do what we can to be able to\n> benefit from the syscall, but we also won't loop forever if there is a\n> real permission issue.\n>\n> This fixes a regression introduced with 7333ed17 (setup: convert\n> setup_git_directory_gently_1 et al. to strbuf, 2014-07-28) for paths\n> longer than 127 bytes with components that miss read or execute\n> permissions (e.g. 0711 on /home for privacy reasons); we used a fixed\n> PATH_MAX-sized buffer before.\n>\n> Reported-by: Zenobiusz Kunegunda <zenobiusz.kunegunda@interia.pl>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n\n\nNicely analysed and fixed (or is the right word \"worked around\"?)\n\nThanks, will queue.\n"},{"id":"315501","messageId":"kczrpegpzxhedtxmjptr@skdf","threadId":"45517","inReplyTo":"xmqq1stj4kmp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Zenobiusz Kunegunda","fromEmail":"zenobiusz.kunegunda@interia.pl","sentAt":"2017-03-27T05:55:19Z","receivedAt":"2017-03-27T06:01:55Z","isPatch":true,"sender":{"key":"zenobiusz.kunegunda@interia.pl","avatar":null},"body":"\nOd: \"Junio C Hamano\" <gitster@pobox.com>\nDo: \"René Scharfe\" <l.s.r@web.de>;\nWysłane: 2:40 Poniedziałek 2017-03-27\nTemat: Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD\n\n> \n> Nicely analysed and fixed (or is the right word \"worked around\"?)\n> \n> Thanks, will queue.\n> \n\n\nIs this patch going to be included in next git version ( or sooner ) by any chance?\n\nThank you, everyone,  for your attention to the problem."},{"id":"315523","messageId":"xmqqo9wm1s34.fsf@gitster.mtv.corp.google.com","threadId":"45517","inReplyTo":"kczrpegpzxhedtxmjptr@skdf","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-27T18:40:15Z","receivedAt":"2017-03-27T18:41:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zenobiusz Kunegunda <zenobiusz.kunegunda@interia.pl> writes:\n\n> Od: \"Junio C Hamano\" <gitster@pobox.com>\n> Do: \"René Scharfe\" <l.s.r@web.de>;\n> Wysłane: 2:40 Poniedziałek 2017-03-27\n> Temat: Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD\n>\n>> \n>> Nicely analysed and fixed (or is the right word \"worked around\"?)\n>> \n>> Thanks, will queue.\n>\n> Is this patch going to be included in next git version ( or sooner ) by any chance?\n\nAbsolutely.  Thanks for your initial report and sticking with us\nduring the session to identify the root cause that led to this\nsolution.\n\nAgain, René, thanks for your superb analysis and solution.\n"},{"id":"315621","messageId":"CAP8UFD3pXSf+RhysULQyd2kdKSkBWdoKs+L6GPQ4jwpoeP+yOA@mail.gmail.com","threadId":"45517","inReplyTo":"4026bc3b-2999-9daf-d6ab-10c6d007b1e7@web.de","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-28T21:15:12Z","receivedAt":"2017-03-28T21:15:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n> FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n> based on readdir(3) if it fails for some reason.  The latter requires\n> permissions to read and execute path components, while the former does\n> not.  That means that if our buffer is too small and we're missing\n> rights we could get EACCES, but we may succeed with a bigger buffer.\n>\n> Keep retrying if getcwd(3) indicates lack of permissions until our\n> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n> syscall on FreeBSD anyway.  This way we do what we can to be able to\n> benefit from the syscall, but we also won't loop forever if there is a\n> real permission issue.\n\nSorry to be late and maybe I missed something obvious, but the above\nand the patch seem complex to me compared with something like:\n\ndiff --git a/strbuf.c b/strbuf.c\nindex ace58e7367..25eadcbedc 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n*path, size_t hint)\n int strbuf_getcwd(struct strbuf *sb)\n {\n        size_t oldalloc = sb->alloc;\n-       size_t guessed_len = 128;\n+       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n\n        for (;; guessed_len *= 2) {\n                strbuf_grow(sb, guessed_len);\n"},{"id":"315623","messageId":"CAGZ79kaFhrEFFfmBUUM7rc9RftS3cKLcvJe5M8jkwKL=bdemgQ@mail.gmail.com","threadId":"45517","inReplyTo":"CAP8UFD3pXSf+RhysULQyd2kdKSkBWdoKs+L6GPQ4jwpoeP+yOA@mail.gmail.com","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-28T21:24:21Z","receivedAt":"2017-03-28T21:25:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 28, 2017 at 2:15 PM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n>> FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n>> based on readdir(3) if it fails for some reason.  The latter requires\n>> permissions to read and execute path components, while the former does\n>> not.  That means that if our buffer is too small and we're missing\n>> rights we could get EACCES, but we may succeed with a bigger buffer.\n>>\n>> Keep retrying if getcwd(3) indicates lack of permissions until our\n>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n>> syscall on FreeBSD anyway.  This way we do what we can to be able to\n>> benefit from the syscall, but we also won't loop forever if there is a\n>> real permission issue.\n>\n> Sorry to be late and maybe I missed something obvious, but the above\n> and the patch seem complex to me compared with something like:\n>\n> diff --git a/strbuf.c b/strbuf.c\n> index ace58e7367..25eadcbedc 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n> *path, size_t hint)\n>  int strbuf_getcwd(struct strbuf *sb)\n>  {\n>         size_t oldalloc = sb->alloc;\n> -       size_t guessed_len = 128;\n> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n>\n>         for (;; guessed_len *= 2) {\n>                 strbuf_grow(sb, guessed_len);\n\nFrom f22a76e911 (strbuf: add strbuf_getcwd(), 2014-07-28):\n    Because it doesn't use a fixed-size buffer it supports\n    arbitrarily long paths, provided the platform's getcwd() does as well.\n    At least on Linux and FreeBSD it handles paths longer than PATH_MAX\n    just fine.\n\nSo with your patch, we'd still see the original issue for paths > PATH_MAX\nIIUC.\n\nThanks,\nStefan\n"},{"id":"315624","messageId":"CAP8UFD3sMyJTkoUccC6hV9peT+q3dKyX=ENpPtgjUx+z01EC5A@mail.gmail.com","threadId":"45517","inReplyTo":"CAGZ79kaFhrEFFfmBUUM7rc9RftS3cKLcvJe5M8jkwKL=bdemgQ@mail.gmail.com","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-28T21:47:48Z","receivedAt":"2017-03-28T21:48:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Mar 28, 2017 at 11:24 PM, Stefan Beller <sbeller@google.com> wrote:\n> On Tue, Mar 28, 2017 at 2:15 PM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n>>> FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n>>> based on readdir(3) if it fails for some reason.  The latter requires\n>>> permissions to read and execute path components, while the former does\n>>> not.  That means that if our buffer is too small and we're missing\n>>> rights we could get EACCES, but we may succeed with a bigger buffer.\n>>>\n>>> Keep retrying if getcwd(3) indicates lack of permissions until our\n>>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n>>> syscall on FreeBSD anyway.  This way we do what we can to be able to\n>>> benefit from the syscall, but we also won't loop forever if there is a\n>>> real permission issue.\n>>\n>> Sorry to be late and maybe I missed something obvious, but the above\n>> and the patch seem complex to me compared with something like:\n>>\n>> diff --git a/strbuf.c b/strbuf.c\n>> index ace58e7367..25eadcbedc 100644\n>> --- a/strbuf.c\n>> +++ b/strbuf.c\n>> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n>> *path, size_t hint)\n>>  int strbuf_getcwd(struct strbuf *sb)\n>>  {\n>>         size_t oldalloc = sb->alloc;\n>> -       size_t guessed_len = 128;\n>> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n>>\n>>         for (;; guessed_len *= 2) {\n>>                 strbuf_grow(sb, guessed_len);\n>\n> From f22a76e911 (strbuf: add strbuf_getcwd(), 2014-07-28):\n>     Because it doesn't use a fixed-size buffer it supports\n>     arbitrarily long paths, provided the platform's getcwd() does as well.\n>     At least on Linux and FreeBSD it handles paths longer than PATH_MAX\n>     just fine.\n\nWell René's patch says:\n\n>>> Keep retrying if getcwd(3) indicates lack of permissions until our\n>>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n>>> syscall on FreeBSD anyway.\n\nSo it says that FreeBSD syscall doesn't handle paths longer than PATH_MAX.\n\n> So with your patch, we'd still see the original issue for paths > PATH_MAX\n> IIUC.\n\nAlso, René's patch just adds:\n\n+               if (errno == EACCES && guessed_len < PATH_MAX)\n+                       continue;\n\nso if the length of the path is > PATH_MAX, then guessed_len will have\nto be > PATH_MAX and then René's patch will do nothing.\n"},{"id":"315625","messageId":"20170328214930.bj4etqn5qecv45im@sigill.intra.peff.net","threadId":"45517","inReplyTo":"CAP8UFD3pXSf+RhysULQyd2kdKSkBWdoKs+L6GPQ4jwpoeP+yOA@mail.gmail.com","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-28T21:49:30Z","receivedAt":"2017-03-28T21:50:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 28, 2017 at 11:15:12PM +0200, Christian Couder wrote:\n\n> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n> > FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n> > based on readdir(3) if it fails for some reason.  The latter requires\n> > permissions to read and execute path components, while the former does\n> > not.  That means that if our buffer is too small and we're missing\n> > rights we could get EACCES, but we may succeed with a bigger buffer.\n> >\n> > Keep retrying if getcwd(3) indicates lack of permissions until our\n> > buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n> > syscall on FreeBSD anyway.  This way we do what we can to be able to\n> > benefit from the syscall, but we also won't loop forever if there is a\n> > real permission issue.\n> \n> Sorry to be late and maybe I missed something obvious, but the above\n> and the patch seem complex to me compared with something like:\n> \n> diff --git a/strbuf.c b/strbuf.c\n> index ace58e7367..25eadcbedc 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n> *path, size_t hint)\n>  int strbuf_getcwd(struct strbuf *sb)\n>  {\n>         size_t oldalloc = sb->alloc;\n> -       size_t guessed_len = 128;\n> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n> \n>         for (;; guessed_len *= 2) {\n>                 strbuf_grow(sb, guessed_len);\n\nI think the main reason is just that we do not have to pay the price to\nallocate PATH_MAX-sized buffers when they are rarely used.\n\nI doubt it matters all that much in practice, though.\n\n-Peff\n"},{"id":"315650","messageId":"CAP8UFD1wqW221qxWqMz1jHxDLKCg71a2VBh+wUdV8sMSEN2aZw@mail.gmail.com","threadId":"45517","inReplyTo":"20170328214930.bj4etqn5qecv45im@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-03-29T04:54:37Z","receivedAt":"2017-03-29T04:54:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Mar 28, 2017 at 11:49 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Mar 28, 2017 at 11:15:12PM +0200, Christian Couder wrote:\n>\n>> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n>> > FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n>> > based on readdir(3) if it fails for some reason.  The latter requires\n>> > permissions to read and execute path components, while the former does\n>> > not.  That means that if our buffer is too small and we're missing\n>> > rights we could get EACCES, but we may succeed with a bigger buffer.\n>> >\n>> > Keep retrying if getcwd(3) indicates lack of permissions until our\n>> > buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n>> > syscall on FreeBSD anyway.  This way we do what we can to be able to\n>> > benefit from the syscall, but we also won't loop forever if there is a\n>> > real permission issue.\n>>\n>> Sorry to be late and maybe I missed something obvious, but the above\n>> and the patch seem complex to me compared with something like:\n>>\n>> diff --git a/strbuf.c b/strbuf.c\n>> index ace58e7367..25eadcbedc 100644\n>> --- a/strbuf.c\n>> +++ b/strbuf.c\n>> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n>> *path, size_t hint)\n>>  int strbuf_getcwd(struct strbuf *sb)\n>>  {\n>>         size_t oldalloc = sb->alloc;\n>> -       size_t guessed_len = 128;\n>> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n>>\n>>         for (;; guessed_len *= 2) {\n>>                 strbuf_grow(sb, guessed_len);\n>\n> I think the main reason is just that we do not have to pay the price to\n> allocate PATH_MAX-sized buffers when they are rarely used.\n>\n> I doubt it matters all that much in practice, though.\n\nYeah, I can understand that.\n\nBut I also wonder if René's patch relies on PATH_MAX being a multiple\nof 128 on FreeBSD and what would happen for a path between 129 and\nPATH_MAX if PATH_MAX was something like 254.\n"},{"id":"315799","messageId":"4a25d1f4-a744-9135-93af-7e675680b91e@web.de","threadId":"45517","inReplyTo":"CAP8UFD1wqW221qxWqMz1jHxDLKCg71a2VBh+wUdV8sMSEN2aZw@mail.gmail.com","subject":"Re: [PATCH] strbuf: support long paths w/o read rights in strbuf_getcwd() on FreeBSD","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-03-30T18:01:49Z","receivedAt":"2017-03-30T18:02:24Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.03.2017 um 06:54 schrieb Christian Couder:\n> On Tue, Mar 28, 2017 at 11:49 PM, Jeff King <peff@peff.net> wrote:\n>> On Tue, Mar 28, 2017 at 11:15:12PM +0200, Christian Couder wrote:\n>>\n>>> On Sun, Mar 26, 2017 at 3:43 PM, René Scharfe <l.s.r@web.de> wrote:\n>>>> FreeBSD implements getcwd(3) as a syscall, but falls back to a version\n>>>> based on readdir(3) if it fails for some reason.  The latter requires\n>>>> permissions to read and execute path components, while the former does\n>>>> not.  That means that if our buffer is too small and we're missing\n>>>> rights we could get EACCES, but we may succeed with a bigger buffer.\n>>>>\n>>>> Keep retrying if getcwd(3) indicates lack of permissions until our\n>>>> buffer can fit PATH_MAX bytes, as that's the maximum supported by the\n>>>> syscall on FreeBSD anyway.  This way we do what we can to be able to\n>>>> benefit from the syscall, but we also won't loop forever if there is a\n>>>> real permission issue.\n>>>\n>>> Sorry to be late and maybe I missed something obvious, but the above\n>>> and the patch seem complex to me compared with something like:\n>>>\n>>> diff --git a/strbuf.c b/strbuf.c\n>>> index ace58e7367..25eadcbedc 100644\n>>> --- a/strbuf.c\n>>> +++ b/strbuf.c\n>>> @@ -441,7 +441,7 @@ int strbuf_readlink(struct strbuf *sb, const char\n>>> *path, size_t hint)\n>>>  int strbuf_getcwd(struct strbuf *sb)\n>>>  {\n>>>         size_t oldalloc = sb->alloc;\n>>> -       size_t guessed_len = 128;\n>>> +       size_t guessed_len = PATH_MAX > 128 ? PATH_MAX : 128;\n>>>\n>>>         for (;; guessed_len *= 2) {\n>>>                 strbuf_grow(sb, guessed_len);\n>>\n>> I think the main reason is just that we do not have to pay the price to\n>> allocate PATH_MAX-sized buffers when they are rarely used.\n>>\n>> I doubt it matters all that much in practice, though.\n>\n> Yeah, I can understand that.\n\nRight, using PATH_MAX as initial size (no ?: operator needed) is the \neasiest fix.  And yes, I wanted to avoid wasting memory just to fix an \nodd special case.\n\nGiven that the function isn't called very often the impact is probably \nnot that high either way, but increasing memory usage by 3200% on Linux \njust didn't sit right with me -- even though 4KB isn't much in absolute \nterms, of course.\n\n> But I also wonder if René's patch relies on PATH_MAX being a multiple\n> of 128 on FreeBSD and what would happen for a path between 129 and\n> PATH_MAX if PATH_MAX was something like 254.\n\nWe can use any value bigger than zero to initialize guessed_len. \ngetcwd() won't have any problems fitting e.g. a path with a length of \n130 bytes into a buffer of 256 bytes in the second round of the loop.\n\nRené\n"}]}