Re: [XEN PATCH 5/5] xen/bitops: address violation of MISRA C Rule 5.5

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 4 Jul 2025, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/include/xen/bitops.h: non-compliant function '__test_and_set_bit(int, 
> volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_set_bit'
> xen/include/xen/bitops.h: non-compliant function '__test_and_clear_bit(int, 
> volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_clear_bit'
> xen/include/xen/bitops.h: non-compliant function '__test_and_change_bit(int, 
> volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_change_bit'
> xen/include/xen/bitops.h: non-compliant function 'test_bit(int, const 
> volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro 'test_bit'
> 
> The primary issue stems from the macro and function
> having identical names, which is confusing and
> non-compliant with common coding standards.
> Change the functions names by adding two underscores at the end.
> No functional changes.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>

I think these should also be deviated either using a SAF in-code comment
if possible or a regex in
automation/eclair_analysis/ECLAIR/deviations.ecl and
docs/misra/deviations.rst



> ---
>  xen/include/xen/bitops.h | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
> index a4d31ec02a..b292470ad7 100644
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -120,7 +120,7 @@ static always_inline bool generic_test_bit(int nr, const 
> volatile void *addr)
>  }
>  
>  /**
> - * __test_and_set_bit - Set a bit and return its old value
> + * __test_and_set_bit__ - Set a bit and return its old value
>   * @nr: Bit to set
>   * @addr: Address to count from
>   *
> @@ -129,7 +129,7 @@ static always_inline bool generic_test_bit(int nr, const 
> volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_set_bit(int nr, volatile void *addr)
> +__test_and_set_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_set_bit
>  #define arch__test_and_set_bit generic__test_and_set_bit
> @@ -139,11 +139,11 @@ __test_and_set_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_set_bit(nr, addr) ({             \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
> -    __test_and_set_bit(nr, addr);                   \
> +    __test_and_set_bit__(nr, addr);                 \
>  })
>  
>  /**
> - * __test_and_clear_bit - Clear a bit and return its old value
> + * __test_and_clear_bit__ - Clear a bit and return its old value
>   * @nr: Bit to clear
>   * @addr: Address to count from
>   *
> @@ -152,7 +152,7 @@ __test_and_set_bit(int nr, volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_clear_bit(int nr, volatile void *addr)
> +__test_and_clear_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_clear_bit
>  #define arch__test_and_clear_bit generic__test_and_clear_bit
> @@ -162,11 +162,11 @@ __test_and_clear_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_clear_bit(nr, addr) ({           \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
> -    __test_and_clear_bit(nr, addr);                 \
> +    __test_and_clear_bit__(nr, addr);               \
>  })
>  
>  /**
> - * __test_and_change_bit - Change a bit and return its old value
> + * __test_and_change_bit__ - Change a bit and return its old value
>   * @nr: Bit to change
>   * @addr: Address to count from
>   *
> @@ -175,7 +175,7 @@ __test_and_clear_bit(int nr, volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_change_bit(int nr, volatile void *addr)
> +__test_and_change_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_change_bit
>  #define arch__test_and_change_bit generic__test_and_change_bit
> @@ -185,11 +185,11 @@ __test_and_change_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_change_bit(nr, addr) ({              \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
> -    __test_and_change_bit(nr, addr);                    \
> +    __test_and_change_bit__(nr, addr);                  \
>  })
>  
>  /**
> - * test_bit - Determine whether a bit is set
> + * test_bit__ - Determine whether a bit is set
>   * @nr: bit number to test
>   * @addr: Address to start counting from
>   *
> @@ -197,7 +197,7 @@ __test_and_change_bit(int nr, volatile void *addr)
>   * If two examples of this operation race, one can appear to succeed
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
> -static always_inline bool test_bit(int nr, const volatile void *addr)
> +static always_inline bool test_bit__(int nr, const volatile void *addr)
>  {
>  #ifndef arch_test_bit
>  #define arch_test_bit generic_test_bit
> @@ -207,7 +207,7 @@ static always_inline bool test_bit(int nr, const volatile 
> void *addr)
>  }
>  #define test_bit(nr, addr) ({                           \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
> -    test_bit(nr, addr);                                 \
> +    test_bit__(nr, addr);                               \
>  })
>  
>  /* --------------------- Please tidy above here --------------------- */
> -- 
> 2.43.0
>