Re: [PATCH v4 08/15] luks2: Split idx into three variables: keyslot_key, digest_key, segment_key.

[Patrick Steinhardt]

On Fri, Nov 06, 2020 at 10:44:28PM -0600, Glenn Washburn wrote:
> Introduce new variables keyslot_key, digest_key, and segment_key which
> represent the integer key of the item in the respective associative array
> when looping over the array items. This replaces using a generically
> named variable named "idx" used for all three values.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/luks2.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index d96764a02..1a4034f28 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -259,12 +259,12 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>  {
>    grub_json_t keyslots, keyslot, digests, digest, segments, segment;
>    grub_size_t i, size;
> -  grub_uint64_t idx;
> +  grub_uint64_t keyslot_key, digest_key, segment_key;
>  
>    /* Get nth keyslot */
>    if (grub_json_getvalue (&keyslots, root, "keyslots") ||
>        grub_json_getchild (&keyslot, &keyslots, keyslot_idx) ||
> -      grub_json_getuint64 (&idx, &keyslot, NULL) ||
> +      grub_json_getuint64 (&keyslot_key, &keyslot, NULL) ||
>        grub_json_getchild (&keyslot, &keyslot, 0) ||
>        luks2_parse_keyslot (k, &keyslot))
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
> %"PRIuGRUB_SIZE, keyslot_idx);
> @@ -276,11 +276,12 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>    for (i = 0; i < size; i++)
>      {
>        if (grub_json_getchild (&digest, &digests, i) ||
> +       grub_json_getuint64 (&digest_key, &digest, NULL) ||
>            grub_json_getchild (&digest, &digest, 0) ||
>            luks2_parse_digest (d, &digest))
>       return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest 
> %"PRIuGRUB_SIZE, i);
>  
> -      if ((d->keyslots & (1 << idx)))
> +      if ((d->keyslots & (1 << keyslot_key)))

It's a bit confusing that this commit introduces `digest_key`, which
isn't used at all right now. It makes sense considering its being used
in the subsequent commit, but it would've helped if this was pointed out
in the commit message.

Other than that: Reviewed-by: Patrick Steinhardt <ps@pks.im>

Patrick

>       break;
>      }
>    if (i == size)
> @@ -293,12 +294,12 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> grub_luks2_digest_t *d, grub_luks2_s
>    for (i = 0; i < size; i++)
>      {
>        if (grub_json_getchild (&segment, &segments, i) ||
> -       grub_json_getuint64 (&idx, &segment, NULL) ||
> +       grub_json_getuint64 (&segment_key, &segment, NULL) ||
>         grub_json_getchild (&segment, &segment, 0) ||
>            luks2_parse_segment (s, &segment))
>       return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> %"PRIuGRUB_SIZE, i);
>  
> -      if ((d->segments & (1 << idx)))
> +      if ((d->segments & (1 << segment_key)))
>       break;
>      }
>    if (i == size)
> -- 
> 2.27.0
>

signature.asc
Description: PGP signature