Make cgpt_add error messages a little more verbose

When the cgpt utility complaints about parameter errors, it is
impossible to tell what exactly went wrong. This change consolidates
error definitions and adds a function to convert integer error values
into text messages.

BRANCH=none
BUG=none
TEST=manual
  . emerge-link vbooot_reference
  . copy generated `cgpt' to a Link device
  . run command with wrong arguments with respect to the existing GPT:
   localhost var # ./cgpt add -i 3 -b 3985408 -s 1757184 -t rootfs -l ROOT-A /dev/sda
   ERROR: cgpt add: Starting LBA overlaps
   ERROR: cgpt add: -i 3 -l ROOT-A -b 3985408 -s 1757184 -t 3CB8E202-3B7E-47DD-8A3C-7FF2A13CFCEC
  . on the host, in the chroot in src/platform/vboot_reference run

   $ make && make runtests

   observe all tests succeed

Change-Id: Ibd23ca0430a875f70524adc99e0509b26ae699b2
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/34003
Reviewed-by: Randall Spangler <rspangler@chromium.org>

5 files changed, 105 insertions, 43 deletions

diff --git a/cgpt/cgpt_add.c b/cgpt/cgpt_add.c
index 319754ac..ada323e3 100644
--- a/cgpt/cgpt_add.c
+++ b/cgpt/cgpt_add.c
@@ -232,6 +232,7 @@ int cgpt_add(CgptAddParams *params) {
   int gpt_retval;
   GptEntry *entry, backup;
   uint32_t index;
+  int rv;
 
   if (params == NULL)
     return CGPT_FAILED;
@@ -319,11 +320,13 @@ int cgpt_add(CgptAddParams *params) {
                          GPT_MODIFIED_HEADER2 | GPT_MODIFIED_ENTRIES2);
   UpdateCrc(&drive.gpt);
 
-  // If the modified entry is illegal, recovery it and return error.
-  if (0 != CheckEntries((GptEntry*)drive.gpt.primary_entries,
-                        (GptHeader*)drive.gpt.primary_header)) {
+  rv = CheckEntries((GptEntry*)drive.gpt.primary_entries,
+                    (GptHeader*)drive.gpt.primary_header);
+
+  if (0 != rv) {
+    // If the modified entry is illegal, recover it and return error.
     memcpy(entry, &backup, sizeof(*entry));
-    Error("At least one parameter is not allowed:\n");
+    Error("%s\n", GptErrorText(rv));
     Error(DumpCgptAddParams(params));
     goto bad;
   }
diff --git a/firmware/lib/cgptlib/cgptlib_internal.c b/firmware/lib/cgptlib/cgptlib_internal.c
index a942b1b3..86b2c151 100644
--- a/firmware/lib/cgptlib/cgptlib_internal.c
+++ b/firmware/lib/cgptlib/cgptlib_internal.c
@@ -126,7 +126,7 @@ int CheckEntries(GptEntry* entries, GptHeader* h) {
   crc32 = Crc32((const uint8_t *)entries,
                 h->size_of_entry * h->number_of_entries);
   if (crc32 != h->entries_crc32)
-    return 1;
+    return GPT_ERROR_CRC_CORRUPTED;
 
   /* Check all entries. */
   for (i = 0, entry = entries; i < h->number_of_entries; i++, entry++) {
@@ -140,7 +140,7 @@ int CheckEntries(GptEntry* entries, GptHeader* h) {
     if ((entry->starting_lba < h->first_usable_lba) ||
         (entry->ending_lba > h->last_usable_lba) ||
         (entry->ending_lba < entry->starting_lba))
-      return 1;
+      return GPT_ERROR_OUT_OF_REGION;
 
     /* Entry must not overlap other entries. */
     for (i2 = 0, e2 = entries; i2 < h->number_of_entries; i2++, e2++) {
@@ -149,14 +149,14 @@ int CheckEntries(GptEntry* entries, GptHeader* h) {
 
       if ((entry->starting_lba >= e2->starting_lba) &&
           (entry->starting_lba <= e2->ending_lba))
-        return 1;
+        return GPT_ERROR_START_LBA_OVERLAP;
       if ((entry->ending_lba >= e2->starting_lba) &&
           (entry->ending_lba <= e2->ending_lba))
-        return 1;
+        return GPT_ERROR_END_LBA_OVERLAP;
 
       /* UniqueGuid field must be unique. */
       if (0 == Memcmp(&entry->unique, &e2->unique, sizeof(Guid)))
-        return 1;
+        return GPT_ERROR_DUP_GUID;
     }
   }
 
@@ -357,3 +357,49 @@ void GetCurrentKernelUniqueGuid(GptData *gpt, void *dest) {
   GptEntry* e = entries + gpt->current_kernel;
   Memcpy(dest, &e->unique, sizeof(Guid));
 }
+
+
+const char* GptErrorText(int error_code)
+{
+  switch(error_code) {
+  case GPT_SUCCESS:
+    return "none";
+
+  case GPT_ERROR_NO_VALID_KERNEL:
+    return "Invalid kernel";
+
+  case GPT_ERROR_INVALID_HEADERS:
+    return "Invalid headers";
+
+  case GPT_ERROR_INVALID_ENTRIES:
+    return "Invalid entries";
+
+  case GPT_ERROR_INVALID_SECTOR_SIZE:
+    return "Invalid sector size";
+
+  case GPT_ERROR_INVALID_SECTOR_NUMBER:
+    return "Invalid sector number";
+
+  case GPT_ERROR_INVALID_UPDATE_TYPE:
+    return "Invalid update type";
+
+  case GPT_ERROR_CRC_CORRUPTED:
+    return "Entries' crc corrupted";
+
+  case GPT_ERROR_OUT_OF_REGION:
+    return "Entry outside of valid region";
+
+  case GPT_ERROR_START_LBA_OVERLAP:
+    return "Starting LBA overlaps";
+
+  case GPT_ERROR_END_LBA_OVERLAP:
+    return "Ending LBA overlaps";
+
+  case GPT_ERROR_DUP_GUID:
+    return "Duplicated GUID";
+
+  default:
+    break;
+  };
+  return "Unknown";
+}
diff --git a/firmware/lib/cgptlib/include/cgptlib.h b/firmware/lib/cgptlib/include/cgptlib.h
index 451ecf0b..b6b27c96 100644
--- a/firmware/lib/cgptlib/include/cgptlib.h
+++ b/firmware/lib/cgptlib/include/cgptlib.h
@@ -16,6 +16,11 @@ enum {
   GPT_ERROR_INVALID_SECTOR_SIZE,
   GPT_ERROR_INVALID_SECTOR_NUMBER,
   GPT_ERROR_INVALID_UPDATE_TYPE,
+  GPT_ERROR_CRC_CORRUPTED,
+  GPT_ERROR_OUT_OF_REGION,
+  GPT_ERROR_START_LBA_OVERLAP,
+  GPT_ERROR_END_LBA_OVERLAP,
+  GPT_ERROR_DUP_GUID,
 };
 
 /* Bit masks for GptData.modified field. */
diff --git a/firmware/lib/cgptlib/include/cgptlib_internal.h b/firmware/lib/cgptlib/include/cgptlib_internal.h
index a4e7d9bd..f899eab0 100644
--- a/firmware/lib/cgptlib/include/cgptlib_internal.h
+++ b/firmware/lib/cgptlib/include/cgptlib_internal.h
@@ -119,4 +119,7 @@ int IsKernelEntry(const GptEntry* e);
 /* Copies the current kernel partition's UniquePartitionGuid to the dest */
 void GetCurrentKernelUniqueGuid(GptData *gpt, void *dest);
 
+/* Returns a pointer to text describing the passed in error */
+const char* GptErrorText(int error_code);
+
 #endif /* VBOOT_REFERENCE_CGPTLIB_INTERNAL_H_ */
diff --git a/tests/cgptlib_test.c b/tests/cgptlib_test.c
index 791815e9..3402ac85 100644
--- a/tests/cgptlib_test.c
+++ b/tests/cgptlib_test.c
@@ -5,7 +5,6 @@
 
 #include <string.h>
 
-#include "cgptlib.h"
 #include "cgptlib_internal.h"
 #include "cgptlib_test.h"
 #include "crc32.h"
@@ -576,8 +575,8 @@ static int EntriesCrcTest() {
   EXPECT(0 == CheckEntries(e2, h1));
   gpt->primary_entries[0] ^= 0xa5;  /* just XOR a non-zero value */
   gpt->secondary_entries[TOTAL_ENTRIES_SIZE-1] ^= 0x5a;
-  EXPECT(1 == CheckEntries(e1, h1));
-  EXPECT(1 == CheckEntries(e2, h1));
+  EXPECT(GPT_ERROR_CRC_CORRUPTED == CheckEntries(e1, h1));
+  EXPECT(GPT_ERROR_CRC_CORRUPTED == CheckEntries(e2, h1));
 
   return TEST_OK;
 }
@@ -598,19 +597,19 @@ static int ValidEntryTest() {
   BuildTestGptData(gpt);
   e1[0].starting_lba = h1->first_usable_lba - 1;
   RefreshCrc32(gpt);
-  EXPECT(1 == CheckEntries(e1, h1));
+  EXPECT(GPT_ERROR_OUT_OF_REGION == CheckEntries(e1, h1));
 
   /* error case: entry.EndingLBA > header.LastUsableLBA */
   BuildTestGptData(gpt);
   e1[2].ending_lba = h1->last_usable_lba + 1;
   RefreshCrc32(gpt);
-  EXPECT(1 == CheckEntries(e1, h1));
+  EXPECT(GPT_ERROR_OUT_OF_REGION == CheckEntries(e1, h1));
 
   /* error case: entry.StartingLBA > entry.EndingLBA */
   BuildTestGptData(gpt);
   e1[3].starting_lba = e1[3].ending_lba + 1;
   RefreshCrc32(gpt);
-  EXPECT(1 == CheckEntries(e1, h1));
+  EXPECT(GPT_ERROR_OUT_OF_REGION == CheckEntries(e1, h1));
 
   /* case: non active entry should be ignored. */
   BuildTestGptData(gpt);
@@ -638,30 +637,36 @@ static int OverlappedPartitionTest() {
       uint64_t ending_lba;
     } entries[16];  /* enough for testing. */
   } cases[] = {
-    {0, {{0, 100, 199}}},
-    {0, {{1, 100, 199}}},
-    {0, {{1, 100, 150}, {1, 200, 250}, {1, 300, 350}}},
-    {1, {{1, 200, 299}, {1, 100, 199}, {1, 100, 100}}},
-    {1, {{1, 200, 299}, {1, 100, 199}, {1, 299, 299}}},
-    {0, {{1, 300, 399}, {1, 200, 299}, {1, 100, 199}}},
-    {1, {{1, 100, 199}, {1, 199, 299}, {1, 299, 399}}},
-    {1, {{1, 100, 199}, {1, 200, 299}, {1, 75, 399}}},
-    {1, {{1, 100, 199}, {1, 75, 250}, {1, 200, 299}}},
-    {1, {{1, 75, 150}, {1, 100, 199}, {1, 200, 299}}},
-    {1, {{1, 200, 299}, {1, 100, 199}, {1, 300, 399}, {1, 100, 399}}},
-    {0, {{1, 200, 299}, {1, 100, 199}, {1, 300, 399}, {0, 100, 399}}},
-    {1, {{1, 200, 300}, {1, 100, 200}, {1, 100, 400}, {1, 300, 400}}},
-    {1, {{0, 200, 300}, {1, 100, 200}, {1, 100, 400}, {1, 300, 400}}},
-    {0, {{1, 200, 300}, {1, 100, 199}, {0, 100, 400}, {0, 300, 400}}},
-    {1, {{1, 200, 299}, {1, 100, 199}, {1, 199, 199}}},
-    {0, {{1, 200, 299}, {0, 100, 199}, {1, 199, 199}}},
-    {0, {{1, 200, 299}, {1, 100, 199}, {0, 199, 199}}},
-    {1, {{1, 199, 199}, {1, 200, 200}, {1, 201, 201}, {1, 202, 202},
-         {1, 203, 203}, {1, 204, 204}, {1, 205, 205}, {1, 206, 206},
-         {1, 207, 207}, {1, 208, 208}, {1, 199, 199}}},
-    {0, {{1, 199, 199}, {1, 200, 200}, {1, 201, 201}, {1, 202, 202},
-         {1, 203, 203}, {1, 204, 204}, {1, 205, 205}, {1, 206, 206},
-         {1, 207, 207}, {1, 208, 208}, {0, 199, 199}}},
+    {GPT_SUCCESS, {{0, 100, 199}}},
+    {GPT_SUCCESS, {{1, 100, 199}}},
+    {GPT_SUCCESS, {{1, 100, 150}, {1, 200, 250}, {1, 300, 350}}},
+    {GPT_ERROR_START_LBA_OVERLAP,
+     {{1, 200, 299}, {1, 100, 199}, {1, 100, 100}}},
+    {GPT_ERROR_END_LBA_OVERLAP, {{1, 200, 299}, {1, 100, 199}, {1, 299, 299}}},
+    {GPT_SUCCESS, {{1, 300, 399}, {1, 200, 299}, {1, 100, 199}}},
+    {GPT_ERROR_END_LBA_OVERLAP, {{1, 100, 199}, {1, 199, 299}, {1, 299, 399}}},
+    {GPT_ERROR_START_LBA_OVERLAP, {{1, 100, 199}, {1, 200, 299}, {1, 75, 399}}},
+    {GPT_ERROR_START_LBA_OVERLAP, {{1, 100, 199}, {1, 75, 250}, {1, 200, 299}}},
+    {GPT_ERROR_END_LBA_OVERLAP, {{1, 75, 150}, {1, 100, 199}, {1, 200, 299}}},
+    {GPT_ERROR_START_LBA_OVERLAP,
+     {{1, 200, 299}, {1, 100, 199}, {1, 300, 399}, {1, 100, 399}}},
+    {GPT_SUCCESS, {{1, 200, 299}, {1, 100, 199}, {1, 300, 399}, {0, 100, 399}}},
+    {GPT_ERROR_START_LBA_OVERLAP,
+     {{1, 200, 300}, {1, 100, 200}, {1, 100, 400}, {1, 300, 400}}},
+    {GPT_ERROR_START_LBA_OVERLAP,
+     {{0, 200, 300}, {1, 100, 200}, {1, 100, 400}, {1, 300, 400}}},
+    {GPT_SUCCESS, {{1, 200, 300}, {1, 100, 199}, {0, 100, 400}, {0, 300, 400}}},
+    {GPT_ERROR_END_LBA_OVERLAP,
+     {{1, 200, 299}, {1, 100, 199}, {1, 199, 199}}},
+    {GPT_SUCCESS, {{1, 200, 299}, {0, 100, 199}, {1, 199, 199}}},
+    {GPT_SUCCESS, {{1, 200, 299}, {1, 100, 199}, {0, 199, 199}}},
+    {GPT_ERROR_START_LBA_OVERLAP,
+     {{1, 199, 199}, {1, 200, 200}, {1, 201, 201}, {1, 202, 202},
+      {1, 203, 203}, {1, 204, 204}, {1, 205, 205}, {1, 206, 206},
+      {1, 207, 207}, {1, 208, 208}, {1, 199, 199}}},
+    {GPT_SUCCESS, {{1, 199, 199}, {1, 200, 200}, {1, 201, 201}, {1, 202, 202},
+                   {1, 203, 203}, {1, 204, 204}, {1, 205, 205}, {1, 206, 206},
+                   {1, 207, 207}, {1, 208, 208}, {0, 199, 199}}},
   };
 
 
@@ -1147,22 +1152,22 @@ static int DuplicateUniqueGuidTest() {
       uint32_t unique_guid;
     } entries[16];   /* enough for testing. */
   } cases[] = {
-    {0, {{100, 109, 1, 1},
+    {GPT_SUCCESS, {{100, 109, 1, 1},
          {110, 119, 2, 2},
          {120, 129, 3, 3},
          {130, 139, 4, 4},
       }},
-    {0, {{100, 109, 1, 1},
+    {GPT_SUCCESS, {{100, 109, 1, 1},
          {110, 119, 1, 2},
          {120, 129, 2, 3},
          {130, 139, 2, 4},
       }},
-    {1, {{100, 109, 1, 1},
+    {GPT_ERROR_DUP_GUID, {{100, 109, 1, 1},
          {110, 119, 2, 2},
          {120, 129, 3, 1},
          {130, 139, 4, 4},
       }},
-    {1, {{100, 109, 1, 1},
+    {GPT_ERROR_DUP_GUID, {{100, 109, 1, 1},
          {110, 119, 1, 2},
          {120, 129, 2, 3},
          {130, 139, 2, 2},