grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1 V2] add --partuuid to probe


From: Andrei Borzenkov
Subject: Re: [PATCH 1/1 V2] add --partuuid to probe
Date: Sun, 19 Feb 2017 09:38:28 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0

18.02.2017 18:31, Steve Kenton пишет:
> Signed-off-by: Steve Kenton <address@hidden>
> ---
> V2 coding style changes as suggested in feedback
> In V1 I was treating p->index as an offset into the partition table instead
> of an index from 0-15 of the partition table slot needing to be multipled
> by the size of a gtp partition table entry. Is this correct? There are two

E-h-h ... either it returns the correct GUID or not, right? :)

> temporary comments about the include files used to remove the magic numbers,
> If you would prefer others just let me know.
> 
>  grub-core/commands/probe.c | 59 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index cf2793e..662c0a3 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -16,6 +16,7 @@
>   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <stddef.h>
>  #include <grub/types.h>
>  #include <grub/misc.h>
>  #include <grub/mm.h>
> @@ -24,6 +25,8 @@
>  #include <grub/device.h>
>  #include <grub/disk.h>
>  #include <grub/partition.h>
> +#include <grub/gpt_partition.h>
> +#include <grub/i386/pc/boot.h>
>  #include <grub/net.h>
>  #include <grub/fs.h>
>  #include <grub/file.h>
> @@ -45,6 +48,7 @@ static const struct grub_arg_option options[] =
>      {"fs",           'f', 0, N_("Determine filesystem type."), 0, 0},
>      {"fs-uuid",              'u', 0, N_("Determine filesystem UUID."), 0, 0},
>      {"label",                'l', 0, N_("Determine filesystem label."), 0, 
> 0},
> +    {"partuuid",     'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* 
> GUID but Linux kernel calls it "PARTUUID" */
>      {0, 0, 0, 0, 0, 0}
>    };
>  
> @@ -154,6 +158,61 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>        grub_device_close (dev);
>        return GRUB_ERR_NONE;
>      }
> +  if (state[6].set)
> +    {
> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
> +      grub_uint8_t diskbuf[16];
> +      if (dev->disk && dev->disk->partition)
> +     {
> +       grub_partition_t p = dev->disk->partition;
> +       if (grub_strcmp (p->partmap->name, "msdos") == 0)

As discussed in other thread, we probably need to skip nested partitions
here.

> +         {
> +           /* 440 location of NT Volume Label in MBR 
> ./include/grub/i386/pc/boot.h */
> +           const int diskid_offset = GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC;
> +           dev->disk->partition = p->parent;
> +           /* little-endian 4-byte NT disk id */
> +           err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
> +           dev->disk->partition = p;
> +           if (err)
> +             return grub_errno;
> +           partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> +                                      diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +                                      p->number + 1); /* one based partition 
> number */
> +         }
> +       else if (grub_strcmp (p->partmap->name, "gpt") == 0)
> +         {
> +           const int guid_offset = p->index * sizeof(struct 
> grub_gpt_partentry) + 

p->index is absolute byte offset from the start of partition entries array.

> +           /* 16 location of GUID in entry ./include/grub/gpt_partition.h */
> +                                   offsetof(struct grub_gpt_partentry, guid);

I scratched my head how it compiled at all and then I noticed that in my
case we have

gcc -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1
-DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem
/usr/lib/gcc/x86_64-linux-gnu/5/include ...

So we do pull in definitions from host even into kernel build (and of
course offset() is defined in stddef.h in this directory).

This looks like something to solve irrespectively; in your case I
suggest you simply read the whole grub_gpt_partentry to avoid.

Although having offsetof() is actually not that bad.


> +           dev->disk->partition = p->parent;
> +           /* little-endian 16-byte EFI partition GUID */
> +           err = grub_disk_read (dev->disk, p->offset, guid_offset, 16, 
> diskbuf);
> +           dev->disk->partition = p;
> +           if (err)
> +             return grub_errno;
> +           partuuid = grub_xasprintf 
> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +                                      diskbuf[3], diskbuf[2], diskbuf[1], 
> diskbuf[0],
> +                                      diskbuf[5], diskbuf[4],
> +                                      diskbuf[7], diskbuf[6],
> +                                      diskbuf[8], diskbuf[9],
> +                                      diskbuf[10], diskbuf[11], diskbuf[12], 
> diskbuf[13], diskbuf[14], diskbuf[15]);
> +         }
> +       else
> +         return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +                            N_("partition map %s does not support partition 
> UUIDs"),
> +                            dev->disk->partition->partmap->name);
> +     }
> +      else
> +     partuuid = grub_strdup (""); /* a freeable empty string */
> +
> +      if (state[0].set)
> +     grub_env_set (state[0].arg, partuuid);
> +      else
> +     grub_printf ("%s", partuuid);
> +      grub_free (partuuid);
> +      grub_device_close (dev);
> +      return GRUB_ERR_NONE;
> +    }
>    grub_device_close (dev);
>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>  }
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]