grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_err


From: Patrick Steinhardt
Subject: Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages.
Date: Fri, 23 Oct 2020 19:52:11 +0200

On Fri, Oct 23, 2020 at 02:08:18PM +0200, Daniel Kiper wrote:
> On Mon, Oct 19, 2020 at 06:09:49PM -0500, Glenn Washburn wrote:
> > When looping over the digests and segments, the loop variable is j, but the
> > variable i is used to index in the the digests and segments json array. The
> > variable i is the keyslot index. Similarly, there are several grub_error()
> > statements using the wrong index in constructing the error string.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/disk/luks2.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > index 31d7166fc..2241e0312 100644
> > --- a/grub-core/disk/luks2.c
> > +++ b/grub-core/disk/luks2.c
> > @@ -275,34 +275,34 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
> > grub_luks2_digest_t *d, grub_luks2_s
> >      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get digests");
> >    for (j = 0; j < size; j++)
> >      {
> > -      if (grub_json_getchild (&digest, &digests, i) ||
> > +      if (grub_json_getchild (&digest, &digests, j) ||
> >            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);
> > +   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest 
> > %"PRIuGRUB_SIZE, j);
> >
> >        if ((d->keyslots & (1 << idx)))
> >     break;
> >      }
> >    if (j == size)
> > -      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> > %"PRIuGRUB_SIZE);
> > +      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
> > %"PRIuGRUB_SIZE, i);
> >
> >    /* Get segment that matches the digest. */
> >    if (grub_json_getvalue (&segments, root, "segments") ||
> >        grub_json_getsize (&size, &segments))
> >      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get segments");
> > -  for (j = 0; j < size; j++)
> > +  for (i = j, j = 0; j < size; j++)
> 
> Should not it be "i = j = 0" instead of "i = j, j = 0"?

The intent is to save the digest index here...

> >      {
> > -      if (grub_json_getchild (&segment, &segments, i) ||
> > +      if (grub_json_getchild (&segment, &segments, j) ||
> >       grub_json_getuint64 (&idx, &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);
> > +   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
> > %"PRIuGRUB_SIZE, j);
> >
> >        if ((d->segments & (1 << idx)))
> >     break;
> >      }
> >    if (j == size)
> > -    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> > %"PRIuGRUB_SIZE);
> > +    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
> > %"PRIuGRUB_SIZE, i);
> 
> s/PRIuGRUB_SIZE, i/PRIuGRUB_SIZE, j/?
> 
> Daniel

... which then gets used here. It initially confused me as well, but it
should be correct. The problem is the awkward naming, but the patch
following this one improves the situation.

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

Patrick

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]