[PATCH v2 4/9] linux: Use getdents64 on non-LFS readdir

[Adhemerval Zanella]

On 19/10/2020 17:50, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 19/10/2020 05:18, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> Using a single allocation make the code slight faster (which not sure if
>>>> plays much difference since a lot of memory copy would be required for 
>>>> non-LFS) and it does not require to handle the possible reallocation
>>>> failure on readdir.
>>>
>>> But you'd still have to handle the case where name is longer than the
>>> allocation, to avoid a buffer overflow.  So an error case still exists.
>>>
>>> Thanks,
>>> Florian
>>
>> The dirstream_ret_entry already handles it by returning EOVERFLOW for such
>> case.  Also keep in mind this limitation is only for the old non-LFS
>> interface, for the LFS one it will use all the available allocated buffer
>> created by the opendir (it still have the 32kb limit for getdents64 call).
>> I can add a more complex buffer handling to resize the auxiliary buffer,
>> but I am not sure it really pays of the complexity.
> 
> Hmm.  The error code should be ENAMETOOLONG.  Ideally, it should be
> delayed until the end of the stream, so that the rest of the directory
> can be listed (similar to what we do for readdir_r).  It's probably okay
> to report an ENOMEM failure immediately.

ENAMETOOLONG does make more sense, I have fixed it locally.  I am not
sure I understood by 'delayed', it it report on the readdir call, but
the next entry will still be reported in the next readdir call.  The
construction such as won't work:

  while (readdir (dp) != NULL)
    {
      [...]
    }

But this will work as intended:

  while (1)
    {
      errno = 0;
      struct dirent *entry = readdir (dp);
      if (entry == NULL)
	{
	  if (errno == ENAMETOOLONG)
	    continue;
	  break;
	}
    }

Even if go to buffer resize, application will still need to handle
ENOMEM so I am not sure if using separated buffer is really an
improvement here (at least on trying to adequate usual way to call
opendir functions).


> 
> Anyway, I'll look at the overflow check and see if we can move this
> forward, and then revisit the translation buffer allocation in a
> separate patch.

Thanks.