Merge pull request #36 from marineam/repair

Fix gptrepair's status bit checking

grub-core/commands/gptprio.c

@@ -91,9 +91,8 @@ grub_find_next (const char *disk_name,
   if (!gpt)
     goto done;
 
-  if ((gpt->status & GRUB_GPT_BOTH_VALID) != GRUB_GPT_BOTH_VALID)
+  if (grub_gpt_repair (dev->disk, gpt))
-    if (grub_gpt_repair (dev->disk, gpt))
+    goto done;
-      goto done;
 
   for (i = 0; (part = grub_gpt_get_partentry (gpt, i)) != NULL; i++)
     {

grub-core/commands/gptrepair.c

@@ -46,8 +46,6 @@ grub_cmd_gptrepair (grub_command_t cmd __attribute__ ((unused)),
   grub_device_t dev = NULL;
   grub_gpt_t gpt = NULL;
   char *dev_name;
-  grub_uint32_t primary_crc, backup_crc;
-  enum grub_gpt_status old_status;
 
   if (argc != 1 || !grub_strlen(args[0]))
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
@@ -67,29 +65,25 @@ grub_cmd_gptrepair (grub_command_t cmd __attribute__ ((unused)),
   if (!gpt)
     goto done;
 
-  primary_crc = gpt->primary.crc32;
+  if (grub_gpt_both_valid (gpt))
-  backup_crc = gpt->backup.crc32;
-  old_status = gpt->status;
-
-  if (grub_gpt_repair (dev->disk, gpt))
-    goto done;
-
-  if (primary_crc == gpt->primary.crc32 &&
-      backup_crc == gpt->backup.crc32 &&
-      old_status && gpt->status)
     {
       grub_printf_ (N_("GPT already valid, %s unmodified.\n"), dev_name);
       goto done;
     }
 
+  if (!grub_gpt_primary_valid (gpt))
+    grub_printf_ (N_("Found invalid primary GPT on %s\n"), dev_name);
+
+  if (!grub_gpt_backup_valid (gpt))
+    grub_printf_ (N_("Found invalid backup GPT on %s\n"), dev_name);
+
+  if (grub_gpt_repair (dev->disk, gpt))
+    goto done;
+
   if (grub_gpt_write (dev->disk, gpt))
     goto done;
 
-  if (!(old_status & GRUB_GPT_PRIMARY_VALID))
+  grub_printf_ (N_("Repaired GPT on %s\n"), dev_name);
-    grub_printf_ (N_("Primary GPT for %s repaired.\n"), dev_name);
-
-  if (!(old_status & GRUB_GPT_BACKUP_VALID))
-    grub_printf_ (N_("Backup GPT for %s repaired.\n"), dev_name);
 
 done:
   if (gpt)

grub-core/lib/gpt.c

@@ -579,6 +579,10 @@ grub_gpt_repair (grub_disk_t disk, grub_gpt_t gpt)
 {
   grub_uint64_t backup_header, backup_entries;
 
+  /* Skip if there is nothing to do.  */
+  if (grub_gpt_both_valid (gpt))
+    return GRUB_ERR_NONE;
+
   grub_dprintf ("gpt", "repairing GPT for %s\n", disk->name);
 
   if (disk->log_sector_size != gpt->log_sector_size)
@@ -631,10 +635,15 @@ grub_gpt_repair (grub_disk_t disk, grub_gpt_t gpt)
   if (grub_gpt_check_primary (gpt))
     return grub_error (GRUB_ERR_BUG, "Generated invalid GPT primary header");
 
+  gpt->status |= (GRUB_GPT_PRIMARY_HEADER_VALID |
+		  GRUB_GPT_PRIMARY_ENTRIES_VALID);
+
   if (grub_gpt_check_backup (gpt))
     return grub_error (GRUB_ERR_BUG, "Generated invalid GPT backup header");
 
-  gpt->status |= GRUB_GPT_BOTH_VALID;
+  gpt->status |= (GRUB_GPT_BACKUP_HEADER_VALID |
+		  GRUB_GPT_BACKUP_ENTRIES_VALID);
+
   grub_dprintf ("gpt", "repairing GPT for %s successful\n", disk->name);
 
   return GRUB_ERR_NONE;
@@ -696,7 +705,7 @@ grub_gpt_write (grub_disk_t disk, grub_gpt_t gpt)
 {
   /* TODO: update/repair protective MBRs too.  */
 
-  if ((gpt->status & GRUB_GPT_BOTH_VALID) != GRUB_GPT_BOTH_VALID)
+  if (!grub_gpt_both_valid (gpt))
     return grub_error (GRUB_ERR_BAD_PART_TABLE, "Invalid GPT data");
 
   grub_dprintf ("gpt", "writing primary GPT to %s\n", disk->name);

include/grub/gpt_partition.h

@@ -161,13 +161,6 @@ typedef enum grub_gpt_status
     GRUB_GPT_BACKUP_ENTRIES_VALID   = 0x20,
   } grub_gpt_status_t;
 
-#define GRUB_GPT_MBR_VALID (GRUB_GPT_PROTECTIVE_MBR|GRUB_GPT_HYBRID_MBR)
-#define GRUB_GPT_PRIMARY_VALID \
-  (GRUB_GPT_PRIMARY_HEADER_VALID|GRUB_GPT_PRIMARY_ENTRIES_VALID)
-#define GRUB_GPT_BACKUP_VALID \
-  (GRUB_GPT_BACKUP_HEADER_VALID|GRUB_GPT_BACKUP_ENTRIES_VALID)
-#define GRUB_GPT_BOTH_VALID (GRUB_GPT_PRIMARY_VALID|GRUB_GPT_BACKUP_VALID)
-
 /* UEFI requires the entries table to be at least 16384 bytes for a
  * total of 128 entries given the standard 128 byte entry size.  */
 #define GRUB_GPT_DEFAULT_ENTRIES_SIZE	16384
@@ -197,6 +190,34 @@ struct grub_gpt
 };
 typedef struct grub_gpt *grub_gpt_t;
 
+/* Helpers for checking the gpt status field.  */
+static inline int
+grub_gpt_mbr_valid (grub_gpt_t gpt)
+{
+  return ((gpt->status & GRUB_GPT_PROTECTIVE_MBR) ||
+	  (gpt->status & GRUB_GPT_HYBRID_MBR));
+}
+
+static inline int
+grub_gpt_primary_valid (grub_gpt_t gpt)
+{
+  return ((gpt->status & GRUB_GPT_PRIMARY_HEADER_VALID) &&
+	  (gpt->status & GRUB_GPT_PRIMARY_ENTRIES_VALID));
+}
+
+static inline int
+grub_gpt_backup_valid (grub_gpt_t gpt)
+{
+  return ((gpt->status & GRUB_GPT_BACKUP_HEADER_VALID) &&
+	  (gpt->status & GRUB_GPT_BACKUP_ENTRIES_VALID));
+}
+
+static inline int
+grub_gpt_both_valid (grub_gpt_t gpt)
+{
+  return grub_gpt_primary_valid (gpt) && grub_gpt_backup_valid (gpt);
+}
+
 /* Translate GPT sectors to GRUB's 512 byte block addresses.  */
 static inline grub_disk_addr_t
 grub_gpt_sector_to_addr (grub_gpt_t gpt, grub_uint64_t sector)