Re: [PATCH] uapi/fcntl: conditionally define AT_RENAME* macros

[Randy Dunlap <rdunlap@infradead.org>]

Hi Amir,


On 8/24/25 10:58 PM, Amir Goldstein wrote:
> On Mon, Aug 25, 2025 at 1:54 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>>
>>
>> On 8/24/25 4:21 PM, Matthew Wilcox wrote:
>>> On Sun, Aug 24, 2025 at 03:10:55PM -0700, Randy Dunlap wrote:
>>>> Don't define the AT_RENAME_* macros when __USE_GNU is defined since
>>>> /usr/include/stdio.h defines them in that case (i.e. when _GNU_SOURCE
>>>> is defined, which causes __USE_GNU to be defined).
>>>>
>>>> Having them defined in 2 places causes build warnings (duplicate
>>>> definitions) in both samples/watch_queue/watch_test.c and
>>>> samples/vfs/test-statx.c.
>>>
>>> It does?  What flags?
>>>
>>
>> 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
>>

>>>
>>> I'm pretty sure C says that duplicate definitions are fine as long
>>> as they're identical.
>> The vales are identical but the strings are not identical.
>>
>> We can't fix stdio.h, but we could just change uapi/linux/fcntl.h
>> to match stdio.h. I suppose.
> 
> I do not specifically object to a patch like this (assuming that is works?):
> 
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -156,9 +156,9 @@
>   */
> 
>  /* 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 AT_RENAME_NOREPLACE    RENAME_NOREPLACE
> +#define AT_RENAME_EXCHANGE     RENAME_EXCHANGE
> +#define AT_RENAME_WHITEOUT     RENAME_WHITEOUT
> 

I'll test that.

> 
> But to be clear, this is a regression introduced by glibc that is likely
> to break many other builds, not only the kernel samples
> and even if we fix linux uapi to conform to its downstream
> copy of definitions, it won't help those users whose programs
> build was broken until they install kernel headers, so feels like you
> should report this regression to glibc and they'd better not "fix" the
> regression by copying the current definition string as that may change as per
> the patch above.
> 

I'll look into that also.

> Why would a library copy definitions from kernel uapi without
> wrapping them with #ifndef or #undef?

To me it looks like they stuck them into the wrong file - stdio.h
instead of fcntl.h.

thanks.
-- 
~Randy