From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kkVVy-0004Wx-51 for mharc-grub-devel@gnu.org; Wed, 02 Dec 2020 12:02:02 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:53662) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kkVVt-0004UC-Np for grub-devel@gnu.org; Wed, 02 Dec 2020 12:01:59 -0500 Received: from dibed.net-space.pl ([84.10.22.86]:34935) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:192) (Exim 4.90_1) (envelope-from ) id 1kkVVr-00040z-Ov for grub-devel@gnu.org; Wed, 02 Dec 2020 12:01:57 -0500 Received: from router-fw.i.net-space.pl ([192.168.52.1]:39506 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S151489AbgLBRBu (ORCPT ); Wed, 2 Dec 2020 18:01:50 +0100 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Wed, 2 Dec 2020 18:01:47 +0100 From: Daniel Kiper To: Glenn Washburn Cc: grub-devel@gnu.org, Patrick Steinhardt Subject: Re: [PATCH v6 01/12] luks2: Add slot_key member to struct grub_luks2_keyslot/segment/digest Message-ID: <20201202170147.sedm2ndxujtqrf2e@tomti.i.net-space.pl> References: <6262aefe9f1fe7e21d1c34518a70cbd3cc15573b.1606467382.git.development@efficientek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6262aefe9f1fe7e21d1c34518a70cbd3cc15573b.1606467382.git.development@efficientek.com> User-Agent: NeoMutt/20170113 (1.7.2) Received-SPF: pass client-ip=84.10.22.86; envelope-from=dkiper@net-space.pl; helo=dibed.net-space.pl X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Dec 2020 17:01:59 -0000 On Fri, Nov 27, 2020 at 03:03:33AM -0600, Glenn Washburn wrote: > This allows code using these structs to know the named key associated with > these json data structures. In the future we can use these to provide better > error messages to the user. > > Get rid of idx variable in luks2_get_keyslot which was overloaded to be used I prefer if you add "()" to the function names, i.e. luks2_get_keyslot(), in the comments and commit messages. This way it is easier to understand what you mean. > for both keyslot and segment slot keys. > > Signed-off-by: Glenn Washburn > --- > grub-core/disk/luks2.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c > index d96764a02..ab2c31dcd 100644 > --- a/grub-core/disk/luks2.c > +++ b/grub-core/disk/luks2.c > @@ -65,6 +65,7 @@ typedef struct grub_luks2_header grub_luks2_header_t; > > struct grub_luks2_keyslot > { > + grub_uint64_t slot_key; Could you be more consistent and use keyslot instead of slot_key here? > grub_int64_t key_size; > grub_int64_t priority; > struct > @@ -103,6 +104,7 @@ typedef struct grub_luks2_keyslot grub_luks2_keyslot_t; > > struct grub_luks2_segment > { > + grub_uint64_t slot_key; Ditto. The code below uses keyslot instead... > grub_uint64_t offset; > const char *size; > const char *encryption; > @@ -112,6 +114,7 @@ typedef struct grub_luks2_segment grub_luks2_segment_t; > > struct grub_luks2_digest > { > + grub_uint64_t slot_key; > /* Both keyslots and segments are interpreted as bitfields here */ > grub_uint64_t keyslots; > grub_uint64_t segments; > @@ -259,12 +262,11 @@ 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; > > /* 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 (&k->slot_key, &keyslot, NULL) || Daniel