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
> > 
> > 
> > y...@sionneau.net wrote,
> > 
> > > From: Yann Sionneau <ysionn...@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 <ysionn...@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-le...@uclibc-ng.org
> > > 
> 
>From 223d46f66af17e18be6e1eb2933a2c70eb64ad47 Mon Sep 17 00:00:00 2001
From: Waldemar Brodkorb <w...@openadk.org>
Date: Fri, 27 Oct 2023 15:42:34 +0200
Subject: [PATCH] depend on __UCLIBC_HAVE_STATX__

Fixes compilation issues on mips64 n32.

Signed-off-by: Waldemar Brodkorb <w...@openadk.org>
---
 libc/sysdeps/linux/common/fstatat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/sysdeps/linux/common/fstatat.c b/libc/sysdeps/linux/common/fstatat.c
index db4a8327f..13611e9d0 100644
--- a/libc/sysdeps/linux/common/fstatat.c
+++ b/libc/sysdeps/linux/common/fstatat.c
@@ -32,7 +32,7 @@ int fstatat(int fd, const char *file, struct stat *buf, int flag)
 libc_hidden_def(fstatat)
 #else
 
-#if defined(__NR_statx)
+#if defined(__NR_statx) && defined __UCLIBC_HAVE_STATX__
 #include <sys/sysmacros.h> // for makedev
 
 int fstatat(int fd, const char *file, struct stat *buf, int flag)
-- 
2.30.2

_______________________________________________
devel mailing list -- devel@uclibc-ng.org
To unsubscribe send an email to devel-le...@uclibc-ng.org

Reply via email to