Re: [PATCH v3 3/4] cryptodisk: Move global variables into grub_cryptomount_args struct

[Patrick Steinhardt]

On Tue, Oct 12, 2021 at 06:26:28PM -0500, Glenn Washburn wrote:
> Signed-off-by: Glenn Washburn <development@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   | 10 ++++++++--
>  5 files changed, 29 insertions(+), 33 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index a5f7b860c..5b38606ed 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;
> -static char *search_uuid;
> -
>  static void
>  cryptodisk_close (grub_cryptodisk_t dev)
>  {
> @@ -1014,7 +1011,7 @@ grub_cryptodisk_scan_device_real (const char *name,
>  
>    FOR_CRYPTODISK_DEVS (cr)
>    {
> -    dev = cr->scan (source, search_uuid, check_boot);
> +    dev = cr->scan (source, cargs);
>      if (grub_errno)
>        return grub_errno;
>      if (!dev)
> @@ -1049,7 +1046,7 @@ grub_cryptodisk_scan_device_real (const char *name,
>  
>      grub_cryptodisk_insert (dev, name, source);
>  
> -    have_it = 1;
> +    cargs->found_uuid = 1;
>  
>      goto cleanup;
>    }
> @@ -1091,7 +1088,7 @@ grub_cryptodisk_cheat_mount (const char *sourcedev, 
> const char *cheat)
>  
>    FOR_CRYPTODISK_DEVS (cr)
>    {
> -    dev = cr->scan (source, search_uuid, check_boot);
> +    dev = cr->scan (source, NULL);

If I didn't get this wrong, then all scan implementations
unconditionally dereference the `grub_cryptomount_args_t` pointer.
So why does this work, and shouldn't we pass down a struct which has the
`search_uuid` and `check_boot` parameters properly set up?

Patrick

signature.asc
Description: PGP signature