bcachefs: Rename struct field swap to prevent macro naming collision

[PATCH] bcachefs: Rename struct field swap to prevent macro naming collision

Posted by Thorsten Blum 1 year, 10 months ago

The struct field swap can collide with the swap() macro defined in
linux/minmax.h. Rename the struct field to prevent such collisions.

Signed-off-by: Thorsten Blum <thorsten.blum@toblux.com>
---
 fs/bcachefs/eytzinger.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/bcachefs/eytzinger.c b/fs/bcachefs/eytzinger.c
index 4ce5e957a6e9..0f955c3c76a7 100644
--- a/fs/bcachefs/eytzinger.c
+++ b/fs/bcachefs/eytzinger.c
@@ -115,7 +115,7 @@ static void swap_bytes(void *a, void *b, size_t n)
 
 struct wrapper {
 	cmp_func_t cmp;
-	swap_func_t swap;
+	swap_func_t swap_func;
 };
 
 /*
@@ -125,7 +125,7 @@ struct wrapper {
 static void do_swap(void *a, void *b, size_t size, swap_r_func_t swap_func, const void *priv)
 {
 	if (swap_func == SWAP_WRAPPER) {
-		((const struct wrapper *)priv)->swap(a, b, (int)size);
+		((const struct wrapper *)priv)->swap_func(a, b, (int)size);
 		return;
 	}
 
@@ -174,7 +174,7 @@ void eytzinger0_sort_r(void *base, size_t n, size_t size,
 	int i, c, r;
 
 	/* called from 'sort' without swap function, let's pick the default */
-	if (swap_func == SWAP_WRAPPER && !((struct wrapper *)priv)->swap)
+	if (swap_func == SWAP_WRAPPER && !((struct wrapper *)priv)->swap_func)
 		swap_func = NULL;
 
 	if (!swap_func) {
@@ -227,7 +227,7 @@ void eytzinger0_sort(void *base, size_t n, size_t size,
 {
 	struct wrapper w = {
 		.cmp  = cmp_func,
-		.swap = swap_func,
+		.swap_func = swap_func,
 	};
 
 	return eytzinger0_sort_r(base, n, size, _CMP_WRAPPER, SWAP_WRAPPER, &w);
-- 
2.44.0

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

Posted by Kent Overstreet 1 year, 10 months ago

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?

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

Posted by Thorsten Blum 1 year, 10 months ago

On 6. Apr 2024, at 20:48, Kent Overstreet <kent.overstreet@linux.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?

Thanks,
Thorsten

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

Posted by Kent Overstreet 1 year, 10 months ago

On Sat, Apr 06, 2024 at 11:21:37PM +0200, Thorsten Blum wrote:
> On 6. Apr 2024, at 20:48, Kent Overstreet <kent.overstreet@linux.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