diff mbox series

[uclibc-ng-devel,1/4] fstatat64: define it as a wrapper of statx if the kernel does not support fstatat64 syscall

Message ID 20230914142941.25389-1-yann@sionneau.net
State Accepted
Headers show
Series [uclibc-ng-devel,1/4] fstatat64: define it as a wrapper of statx if the kernel does not support fstatat64 syscall | expand

Commit Message

Yann Sionneau Sept. 14, 2023, 2:29 p.m. UTC
From: Yann Sionneau <ysionneau@kalray.eu>

Define fstatat64 as a wrapper of statx if the kernel does not support fstatat64 syscall
This is the case for non-legacy architectures that don't define __ARCH_WANT_NEW_STAT 
in their linux arch/xxx/include/asm/unistd.h

Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>

---
 libc/sysdeps/linux/common/fstatat64.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Waldemar Brodkorb Sept. 16, 2023, 12:09 p.m. UTC | #1
Hi Yann,
yann@sionneau.net wrote,

> From: Yann Sionneau <ysionneau@kalray.eu>
> 
> Define fstatat64 as a wrapper of statx if the kernel does not support fstatat64 syscall
> This is the case for non-legacy architectures that don't define __ARCH_WANT_NEW_STAT 
> in their linux arch/xxx/include/asm/unistd.h
> 
> Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>

Series applied and pushed,
 best regards
  Waldemar
Waldemar Brodkorb Oct. 21, 2023, 5:50 a.m. UTC | #2
Hi Yann,

it seems I haven't tested this series good enough.
It breaks mips64n32 build with following error:

libc/sysdeps/linux/common/fstatat.c: In function 'fstatat':
libc/sysdeps/linux/common/fstatat.c:41:22: error: storage size of 'tmp' isn't known
   41 |         struct statx tmp;
      |                      ^~~
In file included from ./include/sys/syscall.h:33,
                 from libc/sysdeps/linux/common/fstatat.c:9:
./include/bits/syscalls.h:290:48: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  290 |         register ARG_TYPE __a1 __asm__("$5") = (ARG_TYPE) arg2;         \
      |                                                ^
./include/bits/syscalls.h:49:9: note: in expansion of macro 'internal_syscall5'
   49 |         internal_syscall##nr (, "li\t$2, %2\t\t\t# " #name "\n\t",      \
      |         ^~~~~~~~~~~~~~~~
./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
   24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
      |                        ^~~~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
   43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
      |               ^~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:44:30: error: 'STATX_BASIC_STATS' undeclared (first use in this function)
   44 |                              STATX_BASIC_STATS, &tmp);
      |                              ^~~~~~~~~~~~~~~~~
./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
  292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
      |                                                           ^~~~
./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
   24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
      |                        ^~~~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
   43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
      |               ^~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:44:30: note: each undeclared identifier is reported only once for each function it appears in
   44 |                              STATX_BASIC_STATS, &tmp);
      |                              ^~~~~~~~~~~~~~~~~
./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
  292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
      |                                                           ^~~~
./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
   24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
      |                        ^~~~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
   43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
      |               ^~~~~~~~~~~~~~
libc/sysdeps/linux/common/fstatat.c:41:22: warning: unused variable 'tmp' [-Wunused-variable]
   41 |         struct statx tmp;
      |                      ^~~
gmake[6]: *** [Makerules:369: libc/sysdeps/linux/common/fstatat.os] Error 1

Any idea?

It is with uClibc-ng current git.

Thanks in advance
 Waldemar


yann@sionneau.net wrote,

> From: Yann Sionneau <ysionneau@kalray.eu>
> 
> Define fstatat64 as a wrapper of statx if the kernel does not support fstatat64 syscall
> This is the case for non-legacy architectures that don't define __ARCH_WANT_NEW_STAT 
> in their linux arch/xxx/include/asm/unistd.h
> 
> Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>
> 
> ---
>  libc/sysdeps/linux/common/fstatat64.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/libc/sysdeps/linux/common/fstatat64.c b/libc/sysdeps/linux/common/fstatat64.c
> index 711521a6a..836ed4114 100644
> --- a/libc/sysdeps/linux/common/fstatat64.c
> +++ b/libc/sysdeps/linux/common/fstatat64.c
> @@ -43,5 +43,28 @@ int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
>  }
>  libc_hidden_def(fstatat64)
>  #else
> +
> +#if defined(__NR_statx) && defined(__UCLIBC_HAVE_STATX__)
> +# include <sys/stat.h>
> +# include <statx_cp.h>
> +# include <fcntl.h> // for AT_NO_AUTOMOUNT
> +
> +int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
> +{
> +	struct statx tmp;
> +
> +	int r = INLINE_SYSCALL(statx, 5, fd, file, AT_NO_AUTOMOUNT | flag,
> +			       STATX_BASIC_STATS, &tmp);
> +
> +	if (r != 0)
> +		return r;
> +
> +	__cp_stat_statx ((struct stat *)buf, &tmp);
> +
> +	return 0;
> +}
> +libc_hidden_def(fstatat64)
> +#endif
> +
>  /* should add emulation with fstat64() and /proc/self/fd/ ... */
>  #endif
> -- 
> 2.42.0
> 
> _______________________________________________
> devel mailing list -- devel@uclibc-ng.org
> To unsubscribe send an email to devel-leave@uclibc-ng.org
>
Yann Sionneau Oct. 21, 2023, 10:10 a.m. UTC | #3
Hi Waldemar,

Oopsy Daisy!

I will try to have a look on monday!

Sorry about that.

Regards,

Yann

Le 21/10/2023 à 07:50, Waldemar Brodkorb a écrit :
> Hi Yann,
>
> it seems I haven't tested this series good enough.
> It breaks mips64n32 build with following error:
>
> libc/sysdeps/linux/common/fstatat.c: In function 'fstatat':
> libc/sysdeps/linux/common/fstatat.c:41:22: error: storage size of 'tmp' isn't known
>     41 |         struct statx tmp;
>        |                      ^~~
> In file included from ./include/sys/syscall.h:33,
>                   from libc/sysdeps/linux/common/fstatat.c:9:
> ./include/bits/syscalls.h:290:48: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>    290 |         register ARG_TYPE __a1 __asm__("$5") = (ARG_TYPE) arg2;         \
>        |                                                ^
> ./include/bits/syscalls.h:49:9: note: in expansion of macro 'internal_syscall5'
>     49 |         internal_syscall##nr (, "li\t$2, %2\t\t\t# " #name "\n\t",      \
>        |         ^~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:44:30: error: 'STATX_BASIC_STATS' undeclared (first use in this function)
>     44 |                              STATX_BASIC_STATS, &tmp);
>        |                              ^~~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
>    292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
>        |                                                           ^~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:44:30: note: each undeclared identifier is reported only once for each function it appears in
>     44 |                              STATX_BASIC_STATS, &tmp);
>        |                              ^~~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
>    292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
>        |                                                           ^~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:41:22: warning: unused variable 'tmp' [-Wunused-variable]
>     41 |         struct statx tmp;
>        |                      ^~~
> gmake[6]: *** [Makerules:369: libc/sysdeps/linux/common/fstatat.os] Error 1
>
> Any idea?
>
> It is with uClibc-ng current git.
>
> Thanks in advance
>   Waldemar
>
>
> yann@sionneau.net wrote,
>
>> From: Yann Sionneau <ysionneau@kalray.eu>
>>
>> Define fstatat64 as a wrapper of statx if the kernel does not support fstatat64 syscall
>> This is the case for non-legacy architectures that don't define __ARCH_WANT_NEW_STAT
>> in their linux arch/xxx/include/asm/unistd.h
>>
>> Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>
>>
>> ---
>>   libc/sysdeps/linux/common/fstatat64.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/libc/sysdeps/linux/common/fstatat64.c b/libc/sysdeps/linux/common/fstatat64.c
>> index 711521a6a..836ed4114 100644
>> --- a/libc/sysdeps/linux/common/fstatat64.c
>> +++ b/libc/sysdeps/linux/common/fstatat64.c
>> @@ -43,5 +43,28 @@ int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
>>   }
>>   libc_hidden_def(fstatat64)
>>   #else
>> +
>> +#if defined(__NR_statx) && defined(__UCLIBC_HAVE_STATX__)
>> +# include <sys/stat.h>
>> +# include <statx_cp.h>
>> +# include <fcntl.h> // for AT_NO_AUTOMOUNT
>> +
>> +int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
>> +{
>> +	struct statx tmp;
>> +
>> +	int r = INLINE_SYSCALL(statx, 5, fd, file, AT_NO_AUTOMOUNT | flag,
>> +			       STATX_BASIC_STATS, &tmp);
>> +
>> +	if (r != 0)
>> +		return r;
>> +
>> +	__cp_stat_statx ((struct stat *)buf, &tmp);
>> +
>> +	return 0;
>> +}
>> +libc_hidden_def(fstatat64)
>> +#endif
>> +
>>   /* should add emulation with fstat64() and /proc/self/fd/ ... */
>>   #endif
>> -- 
>> 2.42.0
>>
>> _______________________________________________
>> devel mailing list -- devel@uclibc-ng.org
>> To unsubscribe send an email to devel-leave@uclibc-ng.org
>>
Yann Sionneau Oct. 21, 2023, 3:06 p.m. UTC | #4
Hi Waldemar,

This is due to statx structure being defined in statx.h

statx.h being included in sys/stat.h conditionally to 
__UCLIBC_HAVE_STATX__ being defined. Which is not, for mips arch.

defining __UCLIBC_HAVE_STATX__ in 
libc/sysdeps/linux/mips/bits/uClibc_arch_features.h does the job and 
everything compiles OK.

BUT, I'm not sure if this is the best way of fixing things...

I am wondering why we use this __UCLIBC_HAVE_STATX__ macro in the libc 
instead of testing the existence of the syscall in the kernel like for 
every other syscall by doing `#if defined(__NR_statx)` ?

Or maybe __UCLIBC_HAVE_STATX__ means something else? Maybe it means we 
want to expose a real statx() function wrapper to userspace?

If this is the case, that would mean fstatat.c would need to include 
directly bits/statx.h instead of sys/stat.h BUT it's explicitly forbidden:

```

#ifndef _SYS_STAT_H
# error Never include <bits/statx.h> directly, include <sys/stat.h> instead.
#endif

```

So... I'm a bit hesitant here on what's the correct thing to do.

Any thoughts on the list?

Regards,

Yann

Le 21/10/2023 à 07:50, Waldemar Brodkorb a écrit :
> Hi Yann,
>
> it seems I haven't tested this series good enough.
> It breaks mips64n32 build with following error:
>
> libc/sysdeps/linux/common/fstatat.c: In function 'fstatat':
> libc/sysdeps/linux/common/fstatat.c:41:22: error: storage size of 'tmp' isn't known
>     41 |         struct statx tmp;
>        |                      ^~~
> In file included from ./include/sys/syscall.h:33,
>                   from libc/sysdeps/linux/common/fstatat.c:9:
> ./include/bits/syscalls.h:290:48: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>    290 |         register ARG_TYPE __a1 __asm__("$5") = (ARG_TYPE) arg2;         \
>        |                                                ^
> ./include/bits/syscalls.h:49:9: note: in expansion of macro 'internal_syscall5'
>     49 |         internal_syscall##nr (, "li\t$2, %2\t\t\t# " #name "\n\t",      \
>        |         ^~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:44:30: error: 'STATX_BASIC_STATS' undeclared (first use in this function)
>     44 |                              STATX_BASIC_STATS, &tmp);
>        |                              ^~~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
>    292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
>        |                                                           ^~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:44:30: note: each undeclared identifier is reported only once for each function it appears in
>     44 |                              STATX_BASIC_STATS, &tmp);
>        |                              ^~~~~~~~~~~~~~~~~
> ./include/bits/syscalls.h:292:59: note: in definition of macro 'internal_syscall5'
>    292 |         register ARG_TYPE __a3 __asm__("$7") = (ARG_TYPE) arg4;         \
>        |                                                           ^~~~
> ./include/bits/syscalls.h:24:24: note: in expansion of macro 'INTERNAL_SYSCALL'
>     24 |      long result_var = INTERNAL_SYSCALL(name, err, nr, args);           \
>        |                        ^~~~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:43:15: note: in expansion of macro 'INLINE_SYSCALL'
>     43 |         ret = INLINE_SYSCALL(statx, 5, fd, file, flag,
>        |               ^~~~~~~~~~~~~~
> libc/sysdeps/linux/common/fstatat.c:41:22: warning: unused variable 'tmp' [-Wunused-variable]
>     41 |         struct statx tmp;
>        |                      ^~~
> gmake[6]: *** [Makerules:369: libc/sysdeps/linux/common/fstatat.os] Error 1
>
> Any idea?
>
> It is with uClibc-ng current git.
>
> Thanks in advance
>   Waldemar
>
>
> yann@sionneau.net wrote,
>
>> From: Yann Sionneau <ysionneau@kalray.eu>
>>
>> Define fstatat64 as a wrapper of statx if the kernel does not support fstatat64 syscall
>> This is the case for non-legacy architectures that don't define __ARCH_WANT_NEW_STAT
>> in their linux arch/xxx/include/asm/unistd.h
>>
>> Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>
>>
>> ---
>>   libc/sysdeps/linux/common/fstatat64.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/libc/sysdeps/linux/common/fstatat64.c b/libc/sysdeps/linux/common/fstatat64.c
>> index 711521a6a..836ed4114 100644
>> --- a/libc/sysdeps/linux/common/fstatat64.c
>> +++ b/libc/sysdeps/linux/common/fstatat64.c
>> @@ -43,5 +43,28 @@ int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
>>   }
>>   libc_hidden_def(fstatat64)
>>   #else
>> +
>> +#if defined(__NR_statx) && defined(__UCLIBC_HAVE_STATX__)
>> +# include <sys/stat.h>
>> +# include <statx_cp.h>
>> +# include <fcntl.h> // for AT_NO_AUTOMOUNT
>> +
>> +int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
>> +{
>> +	struct statx tmp;
>> +
>> +	int r = INLINE_SYSCALL(statx, 5, fd, file, AT_NO_AUTOMOUNT | flag,
>> +			       STATX_BASIC_STATS, &tmp);
>> +
>> +	if (r != 0)
>> +		return r;
>> +
>> +	__cp_stat_statx ((struct stat *)buf, &tmp);
>> +
>> +	return 0;
>> +}
>> +libc_hidden_def(fstatat64)
>> +#endif
>> +
>>   /* should add emulation with fstat64() and /proc/self/fd/ ... */
>>   #endif
>> -- 
>> 2.42.0
>>
>> _______________________________________________
>> devel mailing list -- devel@uclibc-ng.org
>> To unsubscribe send an email to devel-leave@uclibc-ng.org
>>
diff mbox series

Patch

diff --git a/libc/sysdeps/linux/common/fstatat64.c b/libc/sysdeps/linux/common/fstatat64.c
index 711521a6a..836ed4114 100644
--- a/libc/sysdeps/linux/common/fstatat64.c
+++ b/libc/sysdeps/linux/common/fstatat64.c
@@ -43,5 +43,28 @@  int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
 }
 libc_hidden_def(fstatat64)
 #else
+
+#if defined(__NR_statx) && defined(__UCLIBC_HAVE_STATX__)
+# include <sys/stat.h>
+# include <statx_cp.h>
+# include <fcntl.h> // for AT_NO_AUTOMOUNT
+
+int fstatat64(int fd, const char *file, struct stat64 *buf, int flag)
+{
+	struct statx tmp;
+
+	int r = INLINE_SYSCALL(statx, 5, fd, file, AT_NO_AUTOMOUNT | flag,
+			       STATX_BASIC_STATS, &tmp);
+
+	if (r != 0)
+		return r;
+
+	__cp_stat_statx ((struct stat *)buf, &tmp);
+
+	return 0;
+}
+libc_hidden_def(fstatat64)
+#endif
+
 /* should add emulation with fstat64() and /proc/self/fd/ ... */
 #endif