Re: [XEN PATCH v2 1/4] x86: mechanically rename to address MISRA C:2012 Rule 5.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2023 13:15, Jan Beulich wrote:
> On 31.07.2023 09:33, Nicola Vetrini wrote:
>> --- a/xen/arch/x86/include/asm/hvm/save.h
>> +++ b/xen/arch/x86/include/asm/hvm/save.h
>> @@ -49,21 +49,21 @@ void _hvm_read_entry(struct hvm_domain_context *h,
>>   */
>>  #define _hvm_load_entry(_x, _h, _dst, _strict) ({                       \
>>      int r;                                                              \
>> -    struct hvm_save_descriptor *desc                                    \
>> +    struct hvm_save_descriptor *descriptor                              \
>>          = (struct hvm_save_descriptor *)&(_h)->data[(_h)->cur];         \
>>      if ( (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x),                 \
>>                 HVM_SAVE_LENGTH(_x), (_strict))) == 0 )                  \
>>      {                                                                   \
>>          _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));             \
>>          if ( HVM_SAVE_HAS_COMPAT(_x) &&                                 \
>> -             desc->length != HVM_SAVE_LENGTH(_x) )                      \
>> -            r = HVM_SAVE_FIX_COMPAT(_x, (_dst), desc->length);          \
>> +             descriptor->length != HVM_SAVE_LENGTH(_x) )                \
>> +            r = HVM_SAVE_FIX_COMPAT(_x, (_dst), descriptor->length);    \
>>      }                                                                   \
>>      else if (HVM_SAVE_HAS_COMPAT(_x)                                    \
>>               && (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x),          \
>>                         HVM_SAVE_LENGTH_COMPAT(_x), (_strict))) == 0 ) { \
>>          _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH_COMPAT(_x));      \
>> -        r = HVM_SAVE_FIX_COMPAT(_x, (_dst), desc->length);              \
>> +        r = HVM_SAVE_FIX_COMPAT(_x, (_dst), descriptor->length);        \
>>      }                                                                   \
>>      r; })
> 
> The macro-local variable gets too long for my taste, to be honest,
> and it being improperly named anyway suggests it simply wants a
> trailing underscore added. And then, since for a variable named "r"
> the risk of shadowing is equally high, that one wants to gain a
> trailing underscore as well imo. (And while at it, I personally
> would also drop the leading underscores from the macro parameter
> names. Furthermore I think it would be nice if at on the lines
> touched anyway indentation was also corrected. Overall maybe best
> if I submit a patch.)

In that replacement patch I would like to mention what "desc" this
collides with, but your description didn't say so and I'm afraid I
also haven't been able to spot it (grep-ing for "desc", even with
a couple of extra restrictions, still yields way too many hits).

Jan