[PATCH] uapi: Fixup strace compile error

[Guo Ren]

On Mon, Jun 13, 2022 at 9:58 PM Heiko Stübner <heiko at sntech.de> wrote:
>
> Am Montag, 13. Juni 2022, 03:30:51 CEST schrieb guoren at kernel.org:
> > From: Guo Ren <guoren at linux.alibaba.com>
> >
> > There is no CONFIG_64BIT in userspace, we shouldn't limit it with
> > __BITS_PER_LONG == 32 to break the compatibility. Just export F_*64
> > definitions to userspace permanently.
> >
> > gcc-11 -DHAVE_CONFIG_H   -I./linux/x86_64 -I../../../src/linux/x86_64
> > -I./linux/generic -I../../../src/linux/generic -I. -I../../../src
> > -DIN_STRACE=1      -isystem /opt/kernel/include -Wall -Wextra
> > -Wno-missing-field-initializers -Wno-unused-parameter -Wdate-time
> > -Wformat-security -Wimplicit-fallthrough=5 -Winit-self -Wlogical-op
> > -Wmissing-prototypes -Wnested-externs -Wold-style-definition
> > -Wtrampolines -Wundef -Wwrite-strings -Werror   -g -O2 -c -o
> > libstrace_a-fetch_bpf_fprog.o `test -f 'fetch_bpf_fprog.c' || echo
> > '../../../src/'`fetch_bpf_fprog.c
> > In file included from ../../../src/defs.h:404,
> >                  from ../../../src/fcntl.c:12:
> > ../../../src/xlat/fcntlcmds.h:54:7: error: ‘F_GETLK64’ undeclared here
> > (not in a function); did you mean ‘F_GETLK’?
> >    54 |  XLAT(F_GETLK64),
> >       |       ^~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> >    64 | # define XLAT(val)                      { (unsigned)(val), #val
> >       }
> >       |                                                      ^~~
> > ../../../src/xlat/fcntlcmds.h:57:7: error: ‘F_SETLK64’ undeclared here
> > (not in a function); did you mean ‘F_SETLK’?
> >    57 |  XLAT(F_SETLK64),
> >       |       ^~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> >    64 | # define XLAT(val)                      { (unsigned)(val), #val
> >       }
> >       |                                                      ^~~
> > ../../../src/xlat/fcntlcmds.h:60:7: error: ‘F_SETLKW64’ undeclared here
> > (not in a function); did you mean ‘F_SETLKW’?
> >    60 |  XLAT(F_SETLKW64),
> >       |       ^~~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> >    64 | # define XLAT(val)                      { (unsigned)(val), #val
> >       }
> >       |                                                      ^~~
> > make[4]: *** [Makefile:5017: libstrace_a-fcntl.o] Error 1
> >
> > comment by Eugene:
> > Actually, it's quite the opposite: "ifndef" usage made it vailable at all
> > times to the userspace, and this change has actually broken building strace
> > with the latest kernel headers[1][2].  There could be some debate whether
> > having these F_*64 definitions exposed to the user space 64-bit
> > applications, but it seems that were no harm (as they were exposed already
> > for quite some time), and they are useful at least for strace for compat
> > application tracing purposes.
> >
> > Fixes: 306f7cc1e9061 "uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h"
> > Signed-off-by: Guo Ren <guoren at linux.alibaba.com>
> > Reported-by: Eugene Syromiatnikov <esyr at redhat.com>
> > Cc: Christoph Hellwig <hch at lst.de>
> > Cc: Arnd Bergmann <arnd at arndb.de>
> > Cc: Heiko Stuebner <heiko at sntech.de>
> > Cc: Palmer Dabbelt <palmer at rivosinc.com>
> > ---
> >  include/uapi/asm-generic/fcntl.h | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> > index f13d37b60775..cd6bd65ec25d 100644
> > --- a/include/uapi/asm-generic/fcntl.h
> > +++ b/include/uapi/asm-generic/fcntl.h
> > @@ -116,13 +116,11 @@
> >  #define F_GETSIG     11      /* for sockets. */
> >  #endif
> >
> > -#if __BITS_PER_LONG == 32 || defined(__KERNEL__)
> >  #ifndef F_GETLK64
> >  #define F_GETLK64    12      /*  using 'struct flock64' */
> >  #define F_SETLK64    13
> >  #define F_SETLKW64   14
> >  #endif
> > -#endif /* __BITS_PER_LONG == 32 || defined(__KERNEL__) */
>
> Looks like prviously there were a number of ways these constants
> were ifdef'd - or not. A number of platforms already had no ifdef of
> any sort around them before, so this looks like the sane way to do it.
>
> Though in the original patch the special-mips-variant also gained the
>         #if __BITS_PER_LONG == 32 ...
> conditional in arch/mips/include/uapi/asm/fcntl.h .
> So, is it also affected by this issue?
Not sure about mips.

>
>
> Heiko
>
>


-- 
Best Regards
 Guo Ren