On Mon, Sep 27, 2021 at 06:14:02PM -0500, Glenn Washburn wrote: > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > --- > grub-core/disk/cryptodisk.c | 26 +++++++++----------------- > grub-core/disk/geli.c | 9 ++++----- > grub-core/disk/luks.c | 11 +++++------ > grub-core/disk/luks2.c | 6 +++--- > include/grub/cryptodisk.h | 6 ++++-- > 5 files changed, 25 insertions(+), 33 deletions(-) > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index 86eaabe60..5e153ee0a 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -984,9 +984,6 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk) > > #endif > > -static int check_boot, have_it;
We should really just get rid of `have_it`. It's only used in three
locations, and we only require it because
`grub_cryptodisk_scan_device_real` doesn't properly tell us whether it
was found or not. Using a parameter struct to pass back this value to
the caller feels like a code smell, and only papers over this weird
usage.
Anyway, your patch doesn't make it any worse, so we may just as well fix
it after this series has landed.
[snip]
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index 5bd970692..230167ab0 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -69,6 +69,9 @@ typedef gcry_err_code_t
>
> struct grub_cryptomount_args
> {
> + grub_uint32_t check_boot : 1;
> + grub_uint32_t found_uuid : 1;
> + char *search_uuid;
> grub_uint8_t *key_data;
> grub_size_t key_len;
> };
Would be nice to have comments here which explain what these parameters
do. for `key_data` and `key_len` it's obvious enough, but as a reader I
wouldn't really know what `check_boot` ought to indicate.
Patrick
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
