On 28/12/2020 08:42, Adhemerval Zanella wrote:
>>> static idx_t
>>> +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf)
>>> +{
>>> + ssize_t r;
>>> + while (true)
>>> + {
>>> + ptrdiff_t bufsize = buf->length;
>>> + r = __readlink (path, buf->data, bufsize - 1);
>>> + if (r < bufsize - 1)
>>> + break;
>>> + if (!scratch_buffer_grow (buf))
>>> + return -1;
>>> + }
>>> + return r;
>>> +}
>>
>> This function seems to exist because the proposed code calls readlink in two
>> places. Current gnulib has been changed to call it in just one place, so
>> there's less need to split out the function (the splitting complicates
>> out-of-memory checking).
>
> Yes, this trades a stat call by an extra readlink call. However it seems
> that your strategy to use faccessat should be better, assuming it is lighter
> syscall.
Also, it would require a recent kernel (5.8) to avoid faccessat to fallback
to use __NR_faccessat (it would make each call to faccessat2 to issue
two syscall). We can live with this and if it turns a performance issue we
ca the hack to set a global variable to avoid it.
Now this snippet:
47 # include <sysdep.h>
48 # ifdef __ASSUME_FACCESSAT2
49 # define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2
50 # else
51 # define FACCESSAT_NEVER_EOVERFLOWS true
52 # endif
is not required, since Linux faccessat fallback uses LFS stat call that
does not return EOVERFLOW. And it also bleeds Linux implementation details
on generic code.
I will send a patch to fix it.
