On Mon, Dec 07, 2020 at 10:06:44PM -0600, Glenn Washburn wrote: > On Mon, 7 Dec 2020 21:02:39 +0100 > Daniel Kiper <dki...@net-space.pl> wrote: > > > On Sun, Dec 06, 2020 at 02:29:06PM +0100, Patrick Steinhardt wrote: > > > On Fri, Dec 04, 2020 at 10:43:34AM -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 for both keyslot and segment slot keys. > > > > > > > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > > > > > > Personally, I'd have named them `json_slot_idx`. But you've already > > > done so much work on improving the code that I don't want this to > > > be the reason to not give an SOB, especially considering that it's > > > a strict improvement anyway. So: > > > > > > Signed-off-by: Patrick Steinhardt <p...@pks.im> > > > > I can change this to json_slot_idx before committing if Glenn does not > > object. Otherwise Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> > > Thanks Patrick for the sentiment. Looking at the luks2 spec, it says: > > "JSON objects must have their names formatted as a string that > represents a number in the decimal notation (unsigned integer) – > for example ”0”, ”1” and must contain attribute _type_. According to > the _type_, the implementation decides how to handle (or > ignore) such an object. This notation allows mapping to LUKS1 API > functions that use an integer as a reference to keyslots objects." > > So here, the spec is calling that value a "name", and saying that it > must be a string of decimal digits. Looking at the spec, the "name" of > the keyslot object does not need to correspond to the index into the > array of keyslot areas on disk. However it does in the canonical > implementation for use with LUKS1 API functions which require and > integer, as suggested in the quote above. > > I'd say that these numbers are actually an id for the object of its > respective class. In the cryptsetup implementation, the "id" happens > to correspond to the index into the binary key area array, but that's > needn't be the case. My preference would be to name it "id" and second > choice would be just "idx" (since its usually an index into a physical > array of key areas or virtual array of segments and digests). > > I don't think the "json" part of the name is warranted, because it > really has nothing to do with json. The "slot" part is really only > applicable to keyslots because digests and segments don't have an > equivalent slot aspect. So I suggest we name the struct member names > to just "id" instead. And where we just the index of the name-value > pair in the json associative array we use "json_idx" as a suffix. So > this would mean changing the argument keyslot_idx in > luks2_get_keyslot() to keyslot_json_idx. Optionally the local variable > "i" in luks2_get_keyslot() and luks2_parse_segment() could be renamed > to "json_idx" as well (I don't care either way about this though). > > Glenn
Sounds sensible to me. Based on your reasoning, I'm happy with either "id" or "key". So we may just as well just keep it as-is, as you prefer. Patrick
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel