This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Linux: Add <sys/direntries.h>
On 08/07/2019 10:49, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 08/07/2019 08:19, Florian Weimer wrote:
>>> * Florian Weimer:
>>>
>>>> This header file provides the types struct direntry and struct
>>>> direntries, and the functions direntries_init, direntries_read,
>>>> and direntries_next. Using a separate header file (instead of
>>>> augmenting <dirent.h>) allows more straightforward type names
>>>> because identifier collisions are less of a problem (new code
>>>> can work around them).
>>>>
>>>> The d_off member is not exposed via struct direntry because it is
>>>> difficult to use correctly (it refers to the *next* entry), and
>>>> it is also difficult to emulate with other interfaces.
>>>
>>> Any comments on this patch? I think it fits the addition of getdents64.
>>>
>>> <https://sourceware.org/ml/libc-alpha/2019-06/msg00418.html>
>>
>> I really think these function should be part of the application
>> that wants to use getdents instead of glibc. Some of functionalities
>> are already provided by dirent functions and the API adds the extensible
>> layer which in turn adds even more indirection (some getdents usage
>> I see in the wild is exactly to avoid the multiple buffer copies).
>
> How many of these application implementations get the alignment and
> type-punning correct? I agree that this code should be easy to write,
> but it is not so in ISO C.
Probably not many, even now you see on debian sparc maillist programs
that raise alignment issue due sparc restrictions. But masking wrong C
usage is not intention of such interfaces.
>
> I can remove the extension fields. We can probably use symbol
> versioning if the kernel adds another dirent field, similarly to what it
> did for d_type.
>
>> The only advantage I see is it allows use provide a specific buffer
>> for getdents usage instead of the pre-defined one using in opendir.
>> However I would prefer to add a opendir_np which uses a user-provided
>> buffer instead (similar to the hack I did for posix_spawn closeall).
>
> To support 64-bit directory offsets on 32-bit architectures, readdir and
> readdir64 must call malloc after all previous telldir call, to allocate
> space for the 64-to-32 mapping. telldir cannot report failure and
> therefore cannot call malloc. So extending opendir in this way looks
> rather problematic.
I think you meant seekdir instead of telldir. Currently opendir allocate
a large buffer which does not trigger this issue, and we can limit the
minimum workable buffer size for such cases. Also for readddir{64} we do
have the filesystem contraints of the minimum and maximum filename size,
so we might have a estimate buffer size for some operations. I don't see
this as problematic when we document the expected usage of the buffer and
its size constraints.