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

[Daniel Kiper]

On Sun, Nov 15, 2020 at 10:48:37AM +0100, Patrick Steinhardt wrote:
> 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.

Yeah, I think the digest_key change should be dropped from this patch.

Daniel