On Sun, Feb 26, 2017, 16:23 Nicholas Vinson <nvinson...@gmail.com> wrote:
> Optional patch. The goal is to make sure the probe command used in both > the grub core and the Linux userland use similar methods to read the > partition GUIDs. The patch also updates include/grub/gpt_partition.h so > the GUID type struct can be used for both the type GUID and the > partition GUID without using any confusing naming. > > The patch also includes formatting corrections to make sure all changed > lines are no longer than 80 columns. > --- > grub-core/commands/probe.c | 57 > +++++++++++++++++++++++++++++--------------- > grub-core/disk/ldm.c | 2 +- > grub-core/partmap/gpt.c | 4 ++-- > include/grub/gpt_partition.h | 8 +++---- > util/grub-install.c | 2 +- > util/grub-probe.c | 41 +++++++++++++------------------ > 6 files changed, 62 insertions(+), 52 deletions(-) > > diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c > index 5dd1a6bc5..d58ee89aa 100644 > --- a/grub-core/commands/probe.c > +++ b/grub-core/commands/probe.c > @@ -16,7 +16,6 @@ > * 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> > @@ -48,10 +47,25 @@ 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" */ > + /* GUID but Linux kernel calls it "PARTUUID" */ > + {"partuuid", 'g', 0, N_("Determine partition GUID/UUID."), 0, > 0}, > {0, 0, 0, 0, 0, 0} > }; > > +static char * > +sprint_gpt_guid (grub_gpt_part_guid_t guid) > +{ > + guid.data1 = grub_le_to_cpu32 (guid.data1); > + guid.data2 = grub_le_to_cpu16 (guid.data2); > + guid.data3 = grub_le_to_cpu16 (guid.data3); > + > + return grub_xasprintf > ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", > + guid.data1, guid.data2, guid.data3, guid.data4[0], > + guid.data4[1], guid.data4[2], guid.data4[3], > + guid.data4[4], guid.data4[5], guid.data4[6], > + guid.data4[7]); > +} > + > static grub_err_t > grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args) > { > @@ -161,45 +175,50 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int > argc, char **args) > if (state[6].set) > { > char *partuuid = NULL; /* NULL to silence a spurious GCC warning */ > - /* Nested partitions are not supported for now. */ > - /* Non-nested partitions must have dev->disk->partition->parent == > NULL */ > - if (dev->disk && dev->disk->partition && > dev->disk->partition->parent == NULL) > + /* > + * Nested partitions are not supported for now. > + * Non-nested partitions must have dev->disk->partition->parent == > NULL > + */ > I'm not sure that this is enough. In presence of nested partitions Linux may shift extended partitions as well. I think we need to do tests with qemu and bsd-formatted partitions > + if (dev->disk && dev->disk->partition > + && dev->disk->partition->parent == NULL) > { > grub_partition_t p = dev->disk->partition; > if (grub_strcmp (p->partmap->name, "msdos") == 0) > { > - /* little-endian 4-byte NT disk id "GUID" in the MBR */ > - grub_uint8_t diskid[4]; > + /* > + * The partition GUID for MSDOS is the partition number > (starting > + * with 1) prepended with the NT disk signature. > + */ > dev->disk->partition = p->parent; > grub_uint32_t nt_disk_sig; > - err = grub_disk_read (dev->disk, 0, > GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC, sizeof(diskid), diskid); > + err = grub_disk_read (dev->disk, 0, > + GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC, > + sizeof(nt_disk_sig), &nt_disk_sig); > dev->disk->partition = p; > if (err) > return grub_errno; > /* partition numbers are one-based */ > - partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x", > - diskid[3], diskid[2], diskid[1], > disk[0], > + partuuid = grub_xasprintf ("%08x-%02x", > + grub_le_to_cpu32(nt_disk_sig), > p->number + 1); > } > else if (grub_strcmp (p->partmap->name, "gpt") == 0) > { > - struct grub_gpt_partentry e; > + struct grub_gpt_partentry gptdata; > + > dev->disk->partition = p->parent; > - err = grub_disk_read (dev->disk, p->offset, p->index, sizeof > e, &e); > + err = grub_disk_read (dev->disk, p->offset, p->index, > + sizeof gptdata, &gptdata); > 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", > - e.guid[3], e.guid[2], e.guid[1], > e.guid[0], > - e.guid[5], e.guid[4], > - e.guid[7], e.guid[6], > - e.guid[8], e.guid[9], > - e.guid[10], e.guid[11], > e.guid[12], e.guid[13], e.guid[14], e.guid[15]); > + partuuid = sprint_gpt_guid(gptdata.guid); > } > else > return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > - N_("partition map %s does not support > partition UUIDs"), > + N_("partition map %s does not support " > + "partition UUIDs"), > dev->disk->partition->partmap->name); > } > else > diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c > index 0f978ad05..2a22d2d6c 100644 > --- a/grub-core/disk/ldm.c > +++ b/grub-core/disk/ldm.c > @@ -135,7 +135,7 @@ msdos_has_ldm_partition (grub_disk_t dsk) > return has_ldm; > } > > -static const grub_gpt_part_type_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM; > +static const grub_gpt_part_guid_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM; > > /* Helper for gpt_ldm_sector. */ > static int > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c > index 83bcba779..103f6796f 100644 > --- a/grub-core/partmap/gpt.c > +++ b/grub-core/partmap/gpt.c > @@ -33,10 +33,10 @@ static grub_uint8_t grub_gpt_magic[8] = > 0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54 > }; > > -static const grub_gpt_part_type_t grub_gpt_partition_type_empty = > GRUB_GPT_PARTITION_TYPE_EMPTY; > +static const grub_gpt_part_guid_t grub_gpt_partition_type_empty = > GRUB_GPT_PARTITION_TYPE_EMPTY; > > #ifdef GRUB_UTIL > -static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot = > GRUB_GPT_PARTITION_TYPE_BIOS_BOOT; > +static const grub_gpt_part_guid_t grub_gpt_partition_type_bios_boot = > GRUB_GPT_PARTITION_TYPE_BIOS_BOOT; > #endif > > /* 512 << 7 = 65536 byte sectors. */ > diff --git a/include/grub/gpt_partition.h b/include/grub/gpt_partition.h > index 1b32f6725..354fe2246 100644 > --- a/include/grub/gpt_partition.h > +++ b/include/grub/gpt_partition.h > @@ -22,14 +22,14 @@ > #include <grub/types.h> > #include <grub/partition.h> > > -struct grub_gpt_part_type > +struct grub_gpt_part_guid > { > grub_uint32_t data1; > grub_uint16_t data2; > grub_uint16_t data3; > grub_uint8_t data4[8]; > } __attribute__ ((aligned(8))); > -typedef struct grub_gpt_part_type grub_gpt_part_type_t; > +typedef struct grub_gpt_part_guid grub_gpt_part_guid_t; > > #define GRUB_GPT_PARTITION_TYPE_EMPTY \ > { 0x0, 0x0, 0x0, \ > @@ -70,8 +70,8 @@ struct grub_gpt_header > > struct grub_gpt_partentry > { > - grub_gpt_part_type_t type; > - grub_uint8_t guid[16]; > + grub_gpt_part_guid_t type; > + grub_gpt_part_guid_t guid; > grub_uint64_t start; > grub_uint64_t end; > grub_uint64_t attrib; > diff --git a/util/grub-install.c b/util/grub-install.c > index 6c89c2b0c..23b0acb50 100644 > --- a/util/grub-install.c > +++ b/util/grub-install.c > @@ -713,7 +713,7 @@ is_prep_partition (grub_device_t dev) > if (grub_disk_read (dev->disk, p->offset, p->index, > sizeof (gptdata), &gptdata) == 0) > { > - const grub_gpt_part_type_t template = { > + const grub_gpt_part_guid_t template = { > grub_cpu_to_le32_compile_time (0x9e1a2d38), > grub_cpu_to_le16_compile_time (0xc612), > grub_cpu_to_le16_compile_time (0x4316), > diff --git a/util/grub-probe.c b/util/grub-probe.c > index 81f550e0d..3656e32e8 100644 > --- a/util/grub-probe.c > +++ b/util/grub-probe.c > @@ -132,6 +132,20 @@ get_targets_string (void) > return str; > } > > +static int > +print_gpt_guid (grub_gpt_part_guid_t guid) > +{ > + guid.data1 = grub_le_to_cpu32 (guid.data1); > + guid.data2 = grub_le_to_cpu16 (guid.data2); > + guid.data3 = grub_le_to_cpu16 (guid.data3); > + > + return grub_printf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", > + guid.data1, guid.data2, guid.data3, guid.data4[0], > + guid.data4[1], guid.data4[2], guid.data4[3], > + guid.data4[4], guid.data4[5], guid.data4[6], > + guid.data4[7]); > +} > + > static void > do_print (const char *x, void *data) > { > @@ -206,16 +220,7 @@ probe_partuuid (grub_disk_t disk, char delim) > > if (grub_disk_read (disk, p->offset, p->index, > sizeof(gptdata), &gptdata) == 0) > - { > - grub_printf ("%02x%02x%02x%02x-%02x%02x-%02x%02x" > - "-%02x%02x-%02x%02x%02x%02x%02x%02x", > - gptdata.guid[3], gptdata.guid[2], > gptdata.guid[1], > - gptdata.guid[0], gptdata.guid[5], > gptdata.guid[4], > - gptdata.guid[7], gptdata.guid[6], > gptdata.guid[8], > - gptdata.guid[9], gptdata.guid[10], > gptdata.guid[11], > - gptdata.guid[12], gptdata.guid[13], > gptdata.guid[14], > - gptdata.guid[15]); > - } > + print_gpt_guid(gptdata.guid); > disk->partition = p; > } > } > @@ -701,21 +706,7 @@ probe (const char *path, char **device_names, char > delim) > > if (grub_disk_read (dev->disk, p->offset, p->index, > sizeof (gptdata), &gptdata) == 0) > - { > - grub_gpt_part_type_t gpttype; > - gpttype.data1 = grub_le_to_cpu32 (gptdata.type.data1); > - gpttype.data2 = grub_le_to_cpu16 (gptdata.type.data2); > - gpttype.data3 = grub_le_to_cpu16 (gptdata.type.data3); > - grub_memcpy (gpttype.data4, gptdata.type.data4, 8); > - > - grub_printf > ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x", > - gpttype.data1, gpttype.data2, > - gpttype.data3, gpttype.data4[0], > - gpttype.data4[1], gpttype.data4[2], > - gpttype.data4[3], gpttype.data4[4], > - gpttype.data4[5], gpttype.data4[6], > - gpttype.data4[7]); > - } > + print_gpt_guid(gptdata.type); > dev->disk->partition = p; > } > putchar (delim); > -- > 2.12.0 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
