On 9/2/25 2:31 PM, Randy Dunlap wrote:
> Hi,
>
> On 9/1/25 11:58 PM, Amir Goldstein wrote:
>> On Tue, Sep 2, 2025 at 1:14 AM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:
>>>
>>> Define the RENAME_* and AT_RENAME_* macros exactly the same as in
>>> recent glibc <stdio.h> so that duplicate definition build errors in
>>> both samples/watch_queue/watch_test.c and samples/vfs/test-statx.c
>>> no longer happen. When they defined in exactly the same way in
>>> multiple places, the build errors are prevented.
>>>
>>> Defining only the AT_RENAME_* macros is not sufficient since they
>>> depend on the RENAME_* macros, which may not be defined when the
>>> AT_RENAME_* macros are used.
>>>
>>> Build errors being fixed:
>>>
>>> for samples/vfs/test-statx.c:
>>>
>>> In file included from ../samples/vfs/test-statx.c:23:
>>> usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
>>> 159 | #define AT_RENAME_NOREPLACE 0x0001
>>> In file included from ../samples/vfs/test-statx.c:13:
>>> /usr/include/stdio.h:171:10: note: this is the location of the previous definition
>>> 171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
>>> usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
>>> 160 | #define AT_RENAME_EXCHANGE 0x0002
>>> /usr/include/stdio.h:173:10: note: this is the location of the previous definition
>>> 173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
>>> usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
>>> 161 | #define AT_RENAME_WHITEOUT 0x0004
>>> /usr/include/stdio.h:175:10: note: this is the location of the previous definition
>>> 175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>>>
>>> for samples/watch_queue/watch_test.c:
>>>
>>> In file included from usr/include/linux/watch_queue.h:6,
>>> from ../samples/watch_queue/watch_test.c:19:
>>> usr/include/linux/fcntl.h:159:9: warning: ‘AT_RENAME_NOREPLACE’ redefined
>>> 159 | #define AT_RENAME_NOREPLACE 0x0001
>>> In file included from ../samples/watch_queue/watch_test.c:11:
>>> /usr/include/stdio.h:171:10: note: this is the location of the previous definition
>>> 171 | # define AT_RENAME_NOREPLACE RENAME_NOREPLACE
>>> usr/include/linux/fcntl.h:160:9: warning: ‘AT_RENAME_EXCHANGE’ redefined
>>> 160 | #define AT_RENAME_EXCHANGE 0x0002
>>> /usr/include/stdio.h:173:10: note: this is the location of the previous definition
>>> 173 | # define AT_RENAME_EXCHANGE RENAME_EXCHANGE
>>> usr/include/linux/fcntl.h:161:9: warning: ‘AT_RENAME_WHITEOUT’ redefined
>>> 161 | #define AT_RENAME_WHITEOUT 0x0004
>>> /usr/include/stdio.h:175:10: note: this is the location of the previous definition
>>> 175 | # define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>>>
>>> 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 | 9 ++++++---
>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> --- linux-next-20250819.orig/include/uapi/linux/fcntl.h
>>> +++ linux-next-20250819/include/uapi/linux/fcntl.h
>>> @@ -156,9 +156,12 @@
>>> */
>>>
>>> /* Flags for renameat2(2) (must match legacy RENAME_* flags). */
>>> -#define AT_RENAME_NOREPLACE 0x0001
>>> -#define AT_RENAME_EXCHANGE 0x0002
>>> -#define AT_RENAME_WHITEOUT 0x0004
>>> +# define RENAME_NOREPLACE (1 << 0)
>>> +# define AT_RENAME_NOREPLACE RENAME_NOREPLACE
>>> +# define RENAME_EXCHANGE (1 << 1)
>>> +# define AT_RENAME_EXCHANGE RENAME_EXCHANGE
>>> +# define RENAME_WHITEOUT (1 << 2)
>>> +# define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>>>
>>
>> This solution, apart from being terribly wrong (adjust the source to match
>> to value of its downstream copy), does not address the issue that Mathew
>> pointed out on v1 discussion [1]:
>
> I didn't forget or ignore this.
> If the macros have the same values (well, not just values but also the
> same text), then I don't see why it matters whether they are in some older
> version of glibc.
>
>> $ grep -r AT_RENAME_NOREPLACE /usr/include
>> /usr/include/linux/fcntl.h:#define AT_RENAME_NOREPLACE 0x0001
>>
>> It's not in stdio.h at all. This is with libc6 2.41-10
>>
>> [1] https://lore.kernel.org/linux-fsdevel/aKxfGix_o4glz8-Z@xxxxxxxxxxxxxxxxxxxx/
>>
>> I don't know how to resolve the mess that glibc has created.
>
> Yeah, I guess I don't either.
>
>> Perhaps like this:
>>
>> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
>> index f291ab4f94ebc..dde14fa3c2007 100644
>> --- a/include/uapi/linux/fcntl.h
>> +++ b/include/uapi/linux/fcntl.h
>> @@ -155,10 +155,16 @@
>> * as possible, so we can use them for generic bits in the future if necessary.
>> */
>>
>> -/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
>> -#define AT_RENAME_NOREPLACE 0x0001
>> -#define AT_RENAME_EXCHANGE 0x0002
>> -#define AT_RENAME_WHITEOUT 0x0004
>> +/*
>> + * The legacy renameat2(2) RENAME_* flags are conceptually also
>> syscall-specific
>> + * flags, so it could makes sense to create the AT_RENAME_* aliases
>> for them and
>> + * maybe later add support for generic AT_* flags to this syscall.
>> + * However, following a mismatch of definitions in glibc and since no
>> kernel code
>> + * currently uses the AT_RENAME_* aliases, we leave them undefined here.
>> +#define AT_RENAME_NOREPLACE RENAME_NOREPLACE
>> +#define AT_RENAME_EXCHANGE RENAME_EXCHANGE
>> +#define AT_RENAME_WHITEOUT RENAME_WHITEOUT
>> +*/
>
> Well, we do have samples/ code that uses fcntl.h (indirectly; maybe
> that can be fixed).
> See the build errors in the patch description.
>
>
>> /* Flag for faccessat(2). */
>> #define AT_EACCESS 0x200 /* Test access permitted for
>
> With this patch (your suggestion above):
>
> IF a userspace program in samples/ uses <uapi/linux/fcntl.h> without
> using <stdio.h>, [yes, I created one to test this] and without using
> <uapi/linux/fs.h> then the build fails with similar build errors:
>
> ../samples/watch_queue/watch_nostdio.c: In function ‘consumer’:
> ../samples/watch_queue/watch_nostdio.c:33:32: error: ‘RENAME_NOREPLACE’ undeclared (first use in this function)
> 33 | return RENAME_NOREPLACE;
> ../samples/watch_queue/watch_nostdio.c:33:32: note: each undeclared identifier is reported only once for each function it appears in
> ../samples/watch_queue/watch_nostdio.c:37:32: error: ‘RENAME_EXCHANGE’ undeclared (first use in this function)
> 37 | return RENAME_EXCHANGE;
> ../samples/watch_queue/watch_nostdio.c:41:32: error: ‘RENAME_WHITEOUT’ undeclared (first use in this function)
> 41 | return RENAME_WHITEOUT;
>
> This build succeeds with my version 1 patch (full defining of both
> RENAME_* and AT_RENAME_* macros). It fails with the patch that you suggested
> above.
>
> OK, here's what I propose.
>
> a. remove the unused and (sort of) recently added AT_RENAME_* macros
> in include/uapi/linux/fcntl.h. Nothing in the kernel tree uses them.
> This is:
>
> commit b4fef22c2fb9
> Author: Aleksa Sarai <cyphar@xxxxxxxxxx>
> Date: Wed Aug 28 20:19:42 2024 +1000
> uapi: explain how per-syscall AT_* flags should be allocated
>
> These macros should have never been added here IMO.
> Just putting them somewhere as examples (in comments) would be OK.
>
> This alone fixes all of the build errors in samples/ that I originally
> reported.
>
> b. if a userspace program wants to use the RENAME_* macros, it should
> #include <linux/fs.h> instead of <linux/fcntl.h>.
>
> This fixes the "contrived" build error that I manufactured.
>
> Note that some programs in tools/ do use AT_RENAME_* (all 3 macros)
> but they define those macros locally.
>
And after more testing, this is what I think works:
a. remove all of the AT_RENAME-* macros from <uapi/linux/fcntl.h>
(as above)
b. put the AT_RENAME_* macros into <uapi/linux/fs.h> like so:
+/* Flags for renameat2(2) (must match legacy RENAME_* flags). */
+# define AT_RENAME_NOREPLACE RENAME_NOREPLACE
+# define AT_RENAME_EXCHANGE RENAME_EXCHANGE
+# define AT_RENAME_WHITEOUT RENAME_WHITEOUT
so that they match what is in upstream glibc stdio.h, hence not
causing duplicate definition errors.
--
~Randy
