Re: [PATCH v3 1/4] mkimage: Simplify header size logic

[Daniel Kiper]

On Mon, Jan 14, 2019 at 04:27:15PM +0100, Alexander Graf wrote:
> For EFI images, we always have the following layout:
>
>   [PE header]
>   [padding]
>   [first section (which also is the entry point)]
>
> Currently there are 2 places where we define how big header+padding are:
> in the .vaddr_offset member of our target image definition struct as well
> as in code in grub_install_generate_image().
>
> Remove the latter, so that we only have a single place to modify if we
> need to change the padding.
>
> Signed-off-by: Alexander Graf <address@hidden>
> Reviewed-by: Daniel Kiper <address@hidden>
> ---
>  util/mkimage.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 353bb1098..88b991764 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -1226,10 +1226,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>       int header_size;
>       int reloc_addr;
>
> -     if (image_target->voidp_sizeof == 4)
> -       header_size = EFI32_HEADER_SIZE;
> -     else
> -       header_size = EFI64_HEADER_SIZE;
> +     header_size = image_target->vaddr_offset;

After some thinking it seems to me that this patch is wrong and should
be dropped. It, at least, increases needlessly header size. More in the
comments for the other patches.

Daniel