On Thu, 18 Nov 2021 15:25:44 +0100 Daniel Kiper <dki...@net-space.pl> wrote:
> On Tue, Oct 12, 2021 at 06:26:29PM -0500, Glenn Washburn wrote: > > The member found_uuid was never used by the crypto-backends, but was used to > > Ha! Could you make this patch second in this patch series? Then we could > avoid carrying over have_it/found_uuid cruft over succeeding patches. Sure, I was avoiding do that work, but since you've requested it, I'll take a stab at it. I'm thinking I'll make it the first patch though, so its independent from the rest of the series. > > determine if a crypto-backend successfully mounted a cryptodisk with a given > > uuid. This is not needed however, because grub_device_iterate will return 1 > > iff grub_cryptodisk_scan_device returns 1. And grub_cryptodisk_scan_device > > s/iff/if/ "iff" is short hand for "if and only if". I'll expand it. > > will only return 1 if a search_uuid has been specified and a cryptodisk was > > successfully setup by a crypto-backend. So the return value of > > grub_cryptodisk_scan_device is almost equivalent to found_uuid, with the > > exception of the case where a mount is requested or an already opened > > crypto device. > > > > With this change grub_device_iterate will return 1 when > > I like if you add "()" to function names in comments, etc. > > > grub_cryptodisk_scan_device when a crypto device is successfully decrypted > > I think one "when" is redundant here. Or something else is wrong. Indeed, I'll fix this. > > or when the source device has already been successfully opened. Prior to > > this change, trying to mount an already successfully opened device would > > trigger an error with the message "no such cryptodisk found", which is at > > best misleading. The mount should silently succeed in this case, which is > > what happens with this patch. > > > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > > --- > > grub-core/disk/cryptodisk.c | 9 ++++----- > > include/grub/cryptodisk.h | 1 - > > 2 files changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > > index 5b38606ed..8e5277314 100644 > > --- a/grub-core/disk/cryptodisk.c > > +++ b/grub-core/disk/cryptodisk.c > > @@ -1046,8 +1046,6 @@ grub_cryptodisk_scan_device_real (const char *name, > > > > grub_cryptodisk_insert (dev, name, source); > > > > - cargs->found_uuid = 1; > > - > > goto cleanup; > > } > > goto cleanup; > > @@ -1132,7 +1130,7 @@ grub_cryptodisk_scan_device (const char *name, > > > > if (err) > > grub_print_error (); > > - return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0; > > + return (!err && cargs->search_uuid) ? 1 : 0; > > err == GRUB_ERR_NONE && cargs->search_uuid != NULL > > > } > > > > static grub_err_t > > @@ -1152,6 +1150,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int > > argc, char **args) > > > > if (state[0].set) /* uuid */ > > { > > + int found_uuid = 0; > > Zero initialization seems redundant here. It is. I'll remove the initializer. > > grub_cryptodisk_t dev; > > > > dev = grub_cryptodisk_get_by_uuid (args[0]); > > @@ -1164,9 +1163,9 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int > > argc, char **args) > > > > cargs.check_boot = state[2].set; > > cargs.search_uuid = args[0]; > > - grub_device_iterate (&grub_cryptodisk_scan_device, &cargs); > > + found_uuid = grub_device_iterate (&grub_cryptodisk_scan_device, > > &cargs); > > > > - if (!cargs.found_uuid) > > + if (!found_uuid) > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found"); > > return GRUB_ERR_NONE; > > } Glenn _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel