Re: [PATCH v9 6/7] luks2: Add detached header support

[Daniel Kiper]

On Mon, Apr 11, 2022 at 06:40:27AM +0000, Glenn Washburn wrote:
> If a header file is given to the LUKS2 backend, use that file as the LUKS2
> header, instead of looking for it on the disk.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/luks2.c | 67 ++++++++++++++++++++++++++++++------------
>  1 file changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 349462c61a..af5bc4fc82 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -313,13 +313,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>
>  /* Determine whether to use primary or secondary header */
>  static grub_err_t
> -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
> +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file, 
> grub_luks2_header_t *outhdr)
>  {
>    grub_luks2_header_t primary, secondary, *header = &primary;
> -  grub_err_t ret;
> +  grub_err_t ret = GRUB_ERR_NONE;
>
>    /* Read the primary LUKS header. */
> -  ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> +  if (hdr_file)

if (hdr_file != NULL) and below please...

> +    {
> +      if (grub_file_seek (hdr_file, 0) == (grub_off_t) -1)
> +     ret = grub_errno;

Hmmm... Why do not "return grub_errno;"?

> +
> +      else if (grub_file_read (hdr_file, &primary, sizeof (primary)) != 
> sizeof (primary))
> +     ret = grub_errno;

Ditto. And then you can drop "else"...

> +    }
> +  else
> +    ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
>    if (ret)

... and this "if" ... Or to be precise add {} after else and convert to
"if (ret != GRUB_ERR_NONE)".

And it seems to me you can do similar optimizations below.

Daniel