Re: [PATCH] probe: Support probing for msdos PARTUUID

[Daniel Kiper]

On Mon, Sep 30, 2019 at 08:27:59AM -0400, Nicholas Vinson wrote:
> Extend partition UUID probing support in GRUB core to display pseudo partition
> UUIDs for MBR (MSDOS) partitions.
>
> Signed-off-by: Nicholas Vinson <address@hidden>

In general "Reviewed-by: Daniel Kiper <address@hidden>" but one nit below...

> ---
>  grub-core/commands/probe.c | 43 +++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index d2e7f662e..6d79810b4 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -32,6 +32,7 @@
>  #include <grub/env.h>
>  #include <grub/extcmd.h>
>  #include <grub/i18n.h>
> +#include <grub/i386/pc/boot.h>
>
>  GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -104,26 +105,40 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>      {
>        /* AAAABBBB-CCCC-DDDD-EEEE-FFFFFFFFFFFF + null terminator */
>        char val[37] = "none";
> -      if (dev->disk && dev->disk->partition &&
> -       grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> +      if (dev->disk && dev->disk->partition)
>       {
> -       struct grub_gpt_partentry entry;
>         struct grub_partition *p = dev->disk->partition;
>         grub_disk_t disk = grub_disk_open(dev->disk->name);
> +
>         if (!disk)
>           return grub_errno;
> -       if (grub_disk_read(disk, p->offset, p->index, sizeof(entry), &entry))
> -         return grub_errno;
> +       if (grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> +         {
> +           struct grub_gpt_partentry entry;
> +
> +           if (grub_disk_read(disk,
> +                              p->offset, p->index, sizeof(entry), &entry))
> +             return grub_errno;
> +           grub_gpt_part_guid_t *guid = &entry.guid;

I do not like variable definitions intermixed with the code. Please do
not do that. This time I will fix this before committing.

Daniel