Hi Yann,
I added __UCLIBC_HAVE_STATX__ for c-sky when I first added statx
support. What you think about attached patch to fix the mips64 n32
issue? It showed no further regressions while testing.
best regards
Waldemar
Yann Sionneau wrote,
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(a)sionneau.net wrote,
> From: Yann Sionneau <ysionneau(a)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(a)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(a)uclibc-ng.org
> To unsubscribe send an email to devel-leave(a)uclibc-ng.org
>