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

[Jan Beulich <jbeulich@xxxxxxxx>]

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>
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]) )

?

Jan