grub-devel
[Top][All Lists]
Advanced

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

Re: [GRUB PARTUUID PATCH V3 5/5] Harmonize patches


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [GRUB PARTUUID PATCH V3 5/5] Harmonize patches
Date: Mon, 27 Feb 2017 00:34:18 +0000



On Sun, Feb 26, 2017, 16:23 Nicholas Vinson <address@hidden> 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
address@hidden
https://lists.gnu.org/mailman/listinfo/grub-devel

reply via email to

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