Re: [musl] [PATCH] dirent: implement getdents64

[Jiaqing Zhao <jiaqing.zhao@linux.intel.com>]

On 2022-05-24 20:27, Rich Felker wrote:
> On Tue, May 24, 2022 at 04:07:04PM +0800, Jiaqing Zhao wrote:
>> In musl, getdents64 is an alias of getdents, but the type annotation
>> of these two functions are different according to man page[1], causing
>> compile errors. This patch implements the standard getdents64.
>>     ssize_t getdents64(int fd, void *dirp, size_t count);
>>
>> [1] https://man7.org/linux/man-pages/man2/getdents.2.html
>>
>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>> ---
>>  include/dirent.h     | 3 ++-
>>  src/linux/getdents.c | 6 +++++-
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/dirent.h b/include/dirent.h
>> index 650ecf64..4c5367c2 100644
>> --- a/include/dirent.h
>> +++ b/include/dirent.h
>> @@ -11,6 +11,7 @@ extern "C" {
>>  #define __NEED_off_t
>>  #if defined(_BSD_SOURCE) || defined(_GNU_SOURCE)
>>  #define __NEED_size_t
>> +#define __NEED_ssize_t
>>  #endif
>>  
>>  #include <bits/alltypes.h>
>> @@ -50,6 +51,7 @@ long           telldir(DIR *);
>>  #define IFTODT(x) ((x)>>12 & 017)
>>  #define DTTOIF(x) ((x)<<12)
>>  int getdents(int, struct dirent *, size_t);
>> +ssize_t getdents64(int, void *, size_t);
>>  #endif
>>  
>>  #ifdef _GNU_SOURCE
>> @@ -65,7 +67,6 @@ int versionsort(const struct dirent **, const struct dirent **);
>>  #define versionsort64 versionsort
>>  #define off64_t off_t
>>  #define ino64_t ino_t
>> -#define getdents64 getdents
>>  #endif
>>  
>>  #ifdef __cplusplus
>> diff --git a/src/linux/getdents.c b/src/linux/getdents.c
>> index 796c1e5c..923a8076 100644
>> --- a/src/linux/getdents.c
>> +++ b/src/linux/getdents.c
>> @@ -9,4 +9,8 @@ int getdents(int fd, struct dirent *buf, size_t len)
>>  	return syscall(SYS_getdents, fd, buf, len);
>>  }
>>  
>> -weak_alias(getdents, getdents64);
>> +ssize_t getdents64(int fd, void *buf, size_t len)
>> +{
>> +	if (len>INT_MAX) len = INT_MAX;
>> +	return syscall(SYS_getdents64, fd, buf, len);
>> +}
>> -- 
>> 2.34.1
> 
> The inclusion of LFS64 API stuff in musl was a historical mistake that
> resulted from attempting glibc ABI-compat, not a set of interfaces we
> want. What happened was that the weak aliases for glibc ABI compat
> were included, then broken configure scripts that checked only for
> symbol linkage but not for a working declaration "detected" these
> functions as usable, producing miscompiled code from lack of
> declartion. Since it was intended that musl-built programs never
> reference these ABI-compat-only symbols, it was "fixed" by adding the
> macros to redirect them to the correct functions, but this caused lots
> of problems and we're still trying to extricate the mess. If at some
> point the glibc ABI-compat stuff can be moved entirely to the external
> gcompat project, the macros will be removed entirely.
> 
> As such, I think this patch is not appropriate for inclusion.
> 
> Rich

Got your point. In other ABI-compat-symbols, the redirection works as the
function signature are the same, but getdents64 and getdents are not.
A type cast is needed to make them compatible. Modifying the macro can
help solve this difference, will this be an appropriate solution?

#define getdents64(fd, buf, len) getdents((fd), (struct dirent *)(buf), (len))

Jiaqing