On Fri, Sep 5, 2025 at 7:11 AM Aleksa Sarai <cyphar@xxxxxxxxxx> wrote:
>
> On 2025-09-04, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > On Thu, Sep 4, 2025 at 8:22 AM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:
> > >
> > > Don't define the AT_RENAME_* macros at all since the kernel does not
> > > use them nor does the kernel need to provide them for userspace.
> > > Leave them as comments in <uapi/linux/fcntl.h> only as an example.
> > >
> > > The AT_RENAME_* macros have recently been added to glibc's <stdio.h>.
> > > For a kernel allmodconfig build, this made the macros be defined
> > > differently in 2 places (same values but different macro text),
> > > causing build errors/warnings (duplicate definitions) in both
> > > samples/watch_queue/watch_test.c and samples/vfs/test-statx.c.
> > > (<linux/fcntl.h> is included indirecty in both programs above.)
> > >
> > > Fixes: b4fef22c2fb9 ("uapi: explain how per-syscall AT_* flags should be allocated")
> > > Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > > ---
> > > Cc: Amir Goldstein <amir73il@xxxxxxxxx>
> > > Cc: Jeff Layton <jlayton@xxxxxxxxxx>
> > > Cc: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > > Cc: Alexander Aring <alex.aring@xxxxxxxxx>
> > > Cc: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > > Cc: Aleksa Sarai <cyphar@xxxxxxxxxx>
> > > Cc: Jan Kara <jack@xxxxxxx>
> > > Cc: Christian Brauner <brauner@xxxxxxxxxx>
> > > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > > Cc: David Howells <dhowells@xxxxxxxxxx>
> > > CC: linux-api@xxxxxxxxxxxxxxx
> > > To: linux-fsdevel@xxxxxxxxxxxxxxx
> > > ---
> > > include/uapi/linux/fcntl.h | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > --- linux-next-20250819.orig/include/uapi/linux/fcntl.h
> > > +++ linux-next-20250819/include/uapi/linux/fcntl.h
> > > @@ -155,10 +155,16 @@
> > > * as possible, so we can use them for generic bits in the future if necessary.
> > > */
> > >
> > > +/*
> > > + * Note: This is an example of how the AT_RENAME_* flags could be defined,
> > > + * but the kernel has no need to define them, so leave them as comments.
> > > + */
> > > /* Flags for renameat2(2) (must match legacy RENAME_* flags). */
> > > +/*
> > > #define AT_RENAME_NOREPLACE 0x0001
> > > #define AT_RENAME_EXCHANGE 0x0002
> > > #define AT_RENAME_WHITEOUT 0x0004
> > > +*/
> > >
> >
> > I find this end result a bit odd, but I don't want to suggest another variant
> > I already proposed one in v2 review [1] that maybe you did not like.
> > It's fine.
> > I'll let Aleksa and Christian chime in to decide on if and how they want this
> > comment to look or if we should just delete these definitions and be done with
> > this episode.
>
> For my part, I'm fine with these becoming comments or even removing them
> outright. I think that defining them as AT_* flags would've been useful
> examples of how these flags should be used, but it is what it is.
>
> Then again, AT_EXECVE_CHECK went in and used a higher-level bit despite
> the comments describing that this was unfavourable and what should be
> done instead, so maybe attempting to avoid conflicts is an exercise in
> futility...
That's a bummer :-/
but to be fair, AT_EXECVE_CHECK was merged after v23, so I guess
the patch set started way before this comment and got rebased
after the comment was added, so it was easier to miss it.
>
> If it's too much effort to synchronise them between glibc then it's
> better to just close the book on this whole chapter (even though my
> impression is that glibc made a mistake or two when adding the
> definitions).
Considering that glibc has this fix lined up:
https://inbox.sourceware.org/libc-alpha/lhubjnpv03o.fsf@xxxxxxxxxxxxxxxxxxxxxxxx/
Do we need to do anything at all?
Florian,
I am not that familiar with packaging and distributions of glibc
headers and kernel headers to downstream users.
What are the chances that us removing these definitions from the
current kernel header is going to help any downstream user in the future?
Thanks,
Amir.
