Re: [PATCH] xen/ucode: Improve commentary for parsing AMD containers

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/09/2024 6:56 pm, Jan Beulich wrote:
> On 13.09.2024 14:39, Andrew Cooper wrote:
>> Despite writing this code, it's not the easiest logic to follow.
>>
>> Shorten the UCODE_EQUIV_TYPE name, and provide more of an explanation of
>> what's going on.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> I'm okay with this as is, so
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> yet ...
>
>> @@ -335,10 +335,10 @@ static struct microcode_patch *cf_check 
>> cpu_request_microcode(
>>          buf  += 4;
>>          size -= 4;
>>  
>> -        if ( size < sizeof(*et) ||
>> -             (et = buf)->type != UCODE_EQUIV_CPU_TABLE_TYPE ||
>> -             size - sizeof(*et) < et->len ||
>> -             et->len % sizeof(et->eq[0]) )
>> +        if ( size < sizeof(*et) ||                   /* Insufficient space 
>> for header? */
>> +             (et = buf)->type != UCODE_EQUIV_TYPE || /* Not an Equivalence 
>> Table? */
>> +             size - sizeof(*et) < et->len ||         /* Insufficient space 
>> for table? */
>> +             et->len % sizeof(et->eq[0]) )           /* Not a multiple of 
>> equiv_cpu_entry? */
> ... this of course goes quite a bit beyond 80 cols (yet worse for the
> other block further down). How about
>
>
>         if ( /* Insufficient space for header? */
>              size < sizeof(*et) ||
>              /* Not an Equivalence Table? */
>              (et = buf)->type != UCODE_EQUIV_TYPE ||
>              /* Insufficient space for table? */
>              size - sizeof(*et) < et->len ||
>              /* Not a multiple of equiv_cpu_entry? */
>              et->len % sizeof(et->eq[0]) )
>
> ?

That really interferes with legibility.  We explicitly tolerate longer
printk lines for legibility and grepability, and this is the same.

I've managed to reduce the line length a bit with some rewording, but
I'm going put it in with this basic form.

~Andrew