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

[Glenn Washburn]

On Tue, 17 Nov 2020 14:47:39 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:

> 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

Ok, I'll add it to the following, 09, patch in this series.

Glenn