Re: [PATCH v2] x86/gen-cpuid: Avoid violations of Misra rule 1.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.08.2023 15:10, Andrew Cooper wrote:
> Add the script to the X86 section in ./MAINTAINERS.
> 
> Structures or unions without any named members aren't liked by Misra
> (nor the C standard). Avoid emitting such for leaves without any known
> bits.
> 
> The placeholders are affected similarly, but are only visible to MISRA in the
> middle of a patch series adding a new leaf.  The absence of a name was
> intentional as these defines need to not duplicate names.
> 
> As that's not deemed acceptable any more, move placeholder processing into the
> main loop and append the the word number to generate unique names.
> 
>   $ diff cpuid-autogen-{before,after}.h
>   @@ -1034,7 +1034,7 @@
>        bool intel_psfd:1, ipred_ctrl:1, rrsba_ctrl:1, ddp_ctrl:1,     ...
> 
>    #define CPUID_BITFIELD_14 \
>   -    bool :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1,   ...
>   +    uint32_t _placeholder_14
> 
>    #define CPUID_BITFIELD_15 \
>        bool :1, :1, :1, :1, avx_vnni_int8:1, avx_ne_convert:1, :1,    ...
>   @@ -1043,19 +1043,19 @@
>        bool rdcl_no:1, eibrs:1, rsba:1, skip_l1dfl:1, intel_ssb_no:1, ...
> 
>    #define CPUID_BITFIELD_17 \
>   -    bool :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1, :1,   ...
>   +    uint32_t _placeholder_17
> 
>    #define CPUID_BITFIELD_18 \
>   -    uint32_t :32 /* placeholder */
>   +    uint32_t _placeholder_18
> 
>    #define CPUID_BITFIELD_19 \
>   -    uint32_t :32 /* placeholder */
>   +    uint32_t _placeholder_19
> 
>    #define CPUID_BITFIELD_20 \
>   -    uint32_t :32 /* placeholder */
>   +    uint32_t _placeholder_20
> 
>    #define CPUID_BITFIELD_21 \
>   -    uint32_t :32 /* placeholder */
>   +    uint32_t _placeholder_21
> 
>    #endif /* __XEN_X86__FEATURESET_DATA__ */
> 
> No functional change.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one question below.

> v2:
>  * Write it more pythonically.

Yeah, you know I don't speak Python very well. I was glad I got it to
work without overly much hassle.

> @@ -382,7 +382,10 @@ def crunch_numbers(state):
>  
>              names.append(name.lower())
>  
> -        state.bitfields.append("bool " + ":1, ".join(names) + ":1")
> +        if any(names):
> +            state.bitfields.append("bool " + ":1, ".join(names) + ":1")
> +        else:
> +            state.bitfields.append("uint32_t _placeholder_%s" % (word, ))

I thought % could be used here, but then I wouldn't have known to use
%s (elsewhere we use %u), nor to add an empty argument (which I see
is done in a few other places as well, but not uniformly when %s is
used). I assume there's a reason for the specific way you've done it
here?

Jan