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

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 20/10/2020 04:38, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> 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;
>> 	}
>>     }
> 
> I think it's unreasonable to expect that this pattern will work.  It
> assumes that readdir updates dp despite the error.  In other
> implementations, this may produce an endless loop.  This is why for
> readdir_r, we record the ENAMETOOLONG error as pending, and report it
> only at the end.  (We should do the same for EOVERFLOW errors.)

This is what POSIX states as the way to check for errors:

  "Applications wishing to check for error situations should set errno 
   to 0 before calling readdir(). If errno is set to non-zero on return, 
   an error occurred."

What the standard is not clear is whether the position as the directory 
stream is updated in a case of failure.  I think it is a fair assumption
since it also states that 'When the end of the directory is encountered, 
a null pointer shall be returned and errno is not changed.'.  So there
is a distinction between end of directory stream and a failure (in the
example above checking for 'errno == 0' should indicate the end of the
stream).

And it also defines EOVERFLOW as possible transient error.  It
is possible to do for readdir_r because the function return is different
than the returned 'direct' itself, for readdir what we have is the
'errno' to signal such failure.

> 
>> 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).
> 
> ENOMEM is a temporary error, ENAMETOOLONG depends on file system content
> and may be much more difficult to resolve.

Even with ENOMEM, application will require to actually handle a possible
readdir failure and acts accordingly (either by retrying or skip the
file).  But I consider failing with ENOMEM in fact much more confusing,
it exposes internal implementation that needs to be handled by the
application (the internal memory reallocation), where ENAMETOOLONG is
indeed a system limitation that can be resolved by using LFS support.

That's why this limitation is only for non-LFS interface.  This whole fix
is a best effort to a deprecated interface.