Re: [PATCH v8 4/5] efi: Enable booting unified hypervisor/kernel/initrd images

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2020 14:00, Trammell Hudson wrote:
> @@ -1215,9 +1231,11 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>          /* Get the file system interface. */
>          dir_handle = get_parent_handle(loaded_image, &file_name);
>  
> -        /* Read and parse the config file. */
> -        if ( !cfg_file_name )
> +        if ( read_section(loaded_image, L"config", &cfg, NULL) )
> +            PrintStr(L"Using builtin config file\r\n");
> +        else if ( !cfg_file_name )
>          {
> +            /* Read and parse the config file. */

I'm sorry for noticing this only now, but I don't think this comment
should be moved. If no other need for a v9 arises, this can likely
be undone while committing.

> +static bool __init pe_name_compare(const struct PeSectionHeader *sect,
> +                                   const CHAR16 *name)
> +{
> +    size_t i;
> +
> +    if ( sect->Name[0] != '.' )
> +        return -1;

I was about to say "'true' please", but you really mean 'false"
now. (Could perhaps again be fixed while committing.)

> +    for ( i = 1; i < sizeof(sect->Name); i++ )
> +    {
> +        const char c = sect->Name[i];
> +        const CHAR16 cw = name[i - 1];
> +
> +        if ( cw == L'\0' && c == '\0' )
> +            return true;
> +        if ( cw != c )
> +            return false;

Just as a remark (and again spotting only now) this could be had
with one less comparison:

        if ( cw != c )
            return false;
        if ( c == '\0' )
            return true;

At which the need for cw also disappears.

With at least the earlier two issues addressed
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan