Re: [PATCH v2] uapi/fcntl: define RENAME_* and AT_RENAME_* macros

[Randy Dunlap <rdunlap@infradead.org>]

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@infradead.org> 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@infradead.org>
>>> ---
>>> Cc: Amir Goldstein <amir73il@gmail.com>
>>> Cc: Jeff Layton <jlayton@kernel.org>
>>> Cc: Chuck Lever <chuck.lever@oracle.com>
>>> Cc: Alexander Aring <alex.aring@gmail.com>
>>> Cc: Josef Bacik <josef@toxicpanda.com>
>>> Cc: Aleksa Sarai <cyphar@cyphar.com>
>>> Cc: Jan Kara <jack@suse.cz>
>>> Cc: Christian Brauner <brauner@kernel.org>
>>> Cc: Matthew Wilcox <willy@infradead.org>
>>> Cc: David Howells <dhowells@redhat.com>
>>> CC: linux-api@vger.kernel.org
>>> To: linux-fsdevel@vger.kernel.org
>>>
>>>  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@casper.infradead.org/
>>
>> 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@cyphar.com>
> 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