linux-kernel - Re: [PATCH] bcachefs: Rename struct field swap to prevent macro naming collision

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aihoh7frnx5pf2qxlvcntfx4jazdn5m3vnse6q7yqexhqaaul3@luxph6dm5tbz>
Date: Sat, 6 Apr 2024 17:38:09 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Thorsten Blum <thorsten.blum@...lux.com>
Cc: Brian Foster <bfoster@...hat.com>, linux-bcachefs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bcachefs: Rename struct field swap to prevent macro
 naming collision

On Sat, Apr 06, 2024 at 11:21:37PM +0200, Thorsten Blum wrote:
> On 6. Apr 2024, at 20:48, Kent Overstreet <kent.overstreet@...ux.dev> wrote:
> > 
> > On Sat, Apr 06, 2024 at 04:19:20PM +0200, Thorsten Blum wrote:
> >> The struct field swap can collide with the swap() macro defined in
> >> linux/minmax.h. Rename the struct field to prevent such collisions.
> > 
> > Same as in lib/sort.c, so what's the actual reason?
> 
> I included <linux/minmax.h> in arch/m68k/include/asm/bitops.h (to use the min()
> macro), but received the following error:
> 
> fs/bcachefs/eytzinger.c: In function ‘do_swap’:
> fs/bcachefs/eytzinger.c:128:69: error: macro "swap" passed 3 arguments, but takes just 2
>   128 |                 ((const struct wrapper *)priv)->swap(a, b, (int)size);
>       |                                                                     ^
> In file included from ./arch/m68k/include/asm/bitops.h:16,
>                  from ./include/linux/bitops.h:68,
>                  from fs/bcachefs/eytzinger.h:5,
>                  from fs/bcachefs/eytzinger.c:3:
> ./include/linux/minmax.h:270: note: macro "swap" defined here
>   270 | #define swap(a, b) \
>       |
> fs/bcachefs/eytzinger.c:128:47: warning: statement with no effect [-Wunused-value]
>   128 |                 ((const struct wrapper *)priv)->swap(a, b, (int)size);
>       |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
> make[4]: *** [scripts/Makefile.build:244: fs/bcachefs/eytzinger.o] Error 1
> make[3]: *** [scripts/Makefile.build:485: fs/bcachefs] Error 2
> 
> I thought about using #undef swap, but renaming it seemed to be the least
> disruptive change. Maybe there's a better solution?

No it's fine, I was just curious - I'll go ahead and apply it