Re: [PATCH v5 01/13] xen/spinlock: remove misra rule 21.1 violations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.03.2024 08:20, Juergen Gross wrote:
> In xen spinlock code there are several violations of MISRA rule 21.1
> (identifiers starting with "__" or "_[A-Z]").
> 
> Fix them by using trailing underscores instead.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

I can live with the changes as they are, but before giving an ack, I'd
still like to ask if the moved underscores are really useful / necessary
in all cases. E.g.

> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> @@ -22,7 +22,7 @@ union lock_debug {
>          bool unseen:1;
>      };
>  };
> -#define _LOCK_DEBUG { .val = LOCK_DEBUG_INITVAL }
> +#define LOCK_DEBUG_ { .val = LOCK_DEBUG_INITVAL }

... for an internal helper macro it may indeed be better to have a
trailing one here, but ...

> @@ -95,27 +95,27 @@ struct lock_profile_qhead {
>      int32_t                   idx;     /* index for printout */
>  };
>  
> -#define _LOCK_PROFILE(lockname) { .name = #lockname, .lock = &(lockname), }
> -#define _LOCK_PROFILE_PTR(name)                                              
>  \
> -    static struct lock_profile * const __lock_profile_##name                 
>  \
> +#define LOCK_PROFILE_(lockname) { .name = #lockname, .lock = &(lockname), }
> +#define LOCK_PROFILE_PTR_(name)                                              
>  \
> +    static struct lock_profile * const lock_profile__##name                  
>  \

... I'm not entirely convinced of the need for the double infix ones
here ...

> -#define SPIN_LOCK_UNLOCKED _SPIN_LOCK_UNLOCKED(NULL)
> +#define SPIN_LOCK_UNLOCKED SPIN_LOCK_UNLOCKED_(NULL)
>  #define DEFINE_SPINLOCK(l)                                                   
>  \
> -    spinlock_t l = _SPIN_LOCK_UNLOCKED(NULL);                                
>  \
> -    static struct lock_profile __lock_profile_data_##l = _LOCK_PROFILE(l);   
>  \
> -    _LOCK_PROFILE_PTR(l)
> +    spinlock_t l = SPIN_LOCK_UNLOCKED_(NULL);                                
>  \
> +    static struct lock_profile lock_profile_data__##l = LOCK_PROFILE_(l);    
>  \

.... and here.

Jan