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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/08/2023 2:22 pm, Jan Beulich wrote:
> 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>

Thanks.

> 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.

Generally speaking, if you've got bool-like variables around loops,
there's a neater way of expressing it.

This one relies on truthy/falsy values where "" is treated as false. 
(Same too with None, 0, (,), {} and [] for the other primitive datatypes).

>
>> @@ -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?

Hmm.  That was taken from your version.

In python, %s means "call str() on whatever you have".  Similarly, %r
means repr().  str() on an integer behaves as %d.

But yes, %u would be better here, and consistent with the rest of the
script.

Happy to fix on commit.

~Andrew