Re: [PATCH v4 3/4] x86/boot: Rewrite EFI/MBI2 code partly in C

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.09.2024 22:20, Andrew Cooper wrote:
> On 25/09/2024 7:01 am, Frediano Ziglio wrote:
>> +const char * asmlinkage __init
>> +efi_multiboot2_prelude(uint32_t magic, const multiboot2_fixed_t *mbi)
>> +{
>> +    const multiboot2_tag_t *tag;
>> +    EFI_HANDLE ImageHandle = NULL;
>> +    EFI_SYSTEM_TABLE *SystemTable = NULL;
>> +    const char *cmdline = NULL;
>> +    bool have_bs = false;
>> +
>> +    if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC )
>> +        return "ERR: Not a Multiboot2 bootloader!";
>> +
>> +    /* Skip Multiboot2 information fixed part. */
>> +    tag = _p(ROUNDUP((unsigned long)(mbi + 1), MULTIBOOT2_TAG_ALIGN));
>> +
>> +    for ( ; (const void *)tag - (const void *)mbi < mbi->total_size
>> +            && tag->type != MULTIBOOT2_TAG_TYPE_END;
> 
> && on previous line.
> 
> But, this can move into the switch statement and reduce the for()
> expression somewhat.

While it can in principle, it would require further adjustments to the
loop body, which I'm uncertain would be desirable. I notice Frediano
also didn't make any change in v5 for this particular comment. Frediano:
As indicated before, sending a new version without addressing all
comments isn't nice. If you don't agree with a comment and hence don't
make a change requested, you will want to "address" the comment verbally.

Jan