[PATCH] x86: undef REG_IN/REG_OUT to avoid define collisions

[PATCH] x86: undef REG_IN/REG_OUT to avoid define collisions

[Neil Armstrong]

These defines are only used in the inline functions declared
in the arch_hweight.h file, and collides with internal defines
of the Amlogic Mes pinctrl driver when COMPILE_TEST is enabled:

arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant
9 | #define REG_OUT "a"
  |                 ^~~
drivers/pinctrl/meson/pinctrl-meson.h:69:9: note: in expansion of macro ‘REG_OUT’
69 |         REG_OUT,

Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/x86/include/asm/arch_hweight.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
index ba88edd0d58b..139a4b0a2a14 100644
--- a/arch/x86/include/asm/arch_hweight.h
+++ b/arch/x86/include/asm/arch_hweight.h
@@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
 }
 #endif /* CONFIG_X86_32 */
 
+#undef REG_IN
+#undef REG_OUT
+
 #endif
-- 
2.25.1

Re: [PATCH] x86: undef REG_IN/REG_OUT to avoid define collisions

[Dave Hansen]

On 3/30/22 08:28, Neil Armstrong wrote:
> These defines are only used in the inline functions declared
> in the arch_hweight.h file, and collides with internal defines
> of the Amlogic Mes pinctrl driver when COMPILE_TEST is enabled:
> 
> arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant
> 9 | #define REG_OUT "a"
>   |                 ^~~
> drivers/pinctrl/meson/pinctrl-meson.h:69:9: note: in expansion of macro ‘REG_OUT’
> 69 |         REG_OUT,
> 
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/x86/include/asm/arch_hweight.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
> index ba88edd0d58b..139a4b0a2a14 100644
> --- a/arch/x86/include/asm/arch_hweight.h
> +++ b/arch/x86/include/asm/arch_hweight.h
> @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
>  }
>  #endif /* CONFIG_X86_32 */
>  
> +#undef REG_IN
> +#undef REG_OUT

Wouldn't it be a bit less hackish to give these a more qualified name
like HWEIGHT_REG_IN?

Re: [PATCH] x86: undef REG_IN/REG_OUT to avoid define collisions

[Andy Shevchenko]

On Wed, Mar 30, 2022 at 08:33:26AM -0700, Dave Hansen wrote:
> On 3/30/22 08:28, Neil Armstrong wrote:
> > These defines are only used in the inline functions declared
> > in the arch_hweight.h file, and collides with internal defines
> > of the Amlogic Mes pinctrl driver when COMPILE_TEST is enabled:
> > 
> > arch/x86/include/asm/arch_hweight.h:9:17: error: expected identifier before string constant
> > 9 | #define REG_OUT "a"
> >   |                 ^~~
> > drivers/pinctrl/meson/pinctrl-meson.h:69:9: note: in expansion of macro ‘REG_OUT’
> > 69 |         REG_OUT,

Thanks for the patch!

> > Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > ---
> >  arch/x86/include/asm/arch_hweight.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
> > index ba88edd0d58b..139a4b0a2a14 100644
> > --- a/arch/x86/include/asm/arch_hweight.h
> > +++ b/arch/x86/include/asm/arch_hweight.h
> > @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
> >  }
> >  #endif /* CONFIG_X86_32 */
> >  
> > +#undef REG_IN
> > +#undef REG_OUT
> 
> Wouldn't it be a bit less hackish to give these a more qualified name
> like HWEIGHT_REG_IN?

Either way, would it be good to undef them here anyway?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] x86: undef REG_IN/REG_OUT to avoid define collisions

[Thomas Gleixner]

On Wed, Mar 30 2022 at 18:54, Andy Shevchenko wrote:
> On Wed, Mar 30, 2022 at 08:33:26AM -0700, Dave Hansen wrote:
>> On 3/30/22 08:28, Neil Armstrong wrote:
>> > diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h
>> > index ba88edd0d58b..139a4b0a2a14 100644
>> > --- a/arch/x86/include/asm/arch_hweight.h
>> > +++ b/arch/x86/include/asm/arch_hweight.h
>> > @@ -52,4 +52,7 @@ static __always_inline unsigned long __arch_hweight64(__u64 w)
>> >  }
>> >  #endif /* CONFIG_X86_32 */
>> >  
>> > +#undef REG_IN
>> > +#undef REG_OUT
>> 
>> Wouldn't it be a bit less hackish to give these a more qualified name
>> like HWEIGHT_REG_IN?

Absolutely.

> Either way, would it be good to undef them here anyway?

No. Unconditional #undef is a guarantee for hard to diagnose trouble
because it papers over namespace collisions. You can end up with the
wrong constants in your binary which might work well except for the once
in the blue moon corner case.

Been there and stared at such nonsense for weeks...

Name spaces exist for a reason and we are not short of characters here.

Thanks,

        tglx