pnor_partition_table: Make parseTocLine throw exceptions

Change-Id: I66dc8af4fede36c3c952df596040356e11a72644
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

4 files changed, 177 insertions, 48 deletions

diff --git a/mboxd_pnor_partition_table.cpp b/mboxd_pnor_partition_table.cpp
index 84a0314..40d2ea6 100644
--- a/mboxd_pnor_partition_table.cpp
+++ b/mboxd_pnor_partition_table.cpp
@@ -13,31 +13,53 @@ struct vpnor_partition_table
     openpower::virtual_pnor::partition::Table *table = nullptr;
 };
 
-void init_vpnor(struct mbox_context *context)
+int init_vpnor(struct mbox_context *context)
 {
     if (context && !context->vpnor)
     {
+        int rc;
+
         strcpy(context->paths.ro_loc, PARTITION_FILES_RO_LOC);
         strcpy(context->paths.rw_loc, PARTITION_FILES_RW_LOC);
         strcpy(context->paths.prsv_loc, PARTITION_FILES_PRSV_LOC);
         strcpy(context->paths.patch_loc, PARTITION_FILES_PATCH_LOC);
 
-        vpnor_create_partition_table_from_path(context, PARTITION_FILES_RO_LOC);
+        rc = vpnor_create_partition_table_from_path(context,
+                                                    PARTITION_FILES_RO_LOC);
+        if (rc < 0)
+            return rc;
     }
+
+    return 0;
 }
 
-void vpnor_create_partition_table_from_path(struct mbox_context *context,
-                                            const char *path)
+int vpnor_create_partition_table_from_path(struct mbox_context *context,
+                                           const char *path)
 {
-    std::experimental::filesystem::path dir(path);
+    namespace err = sdbusplus::xyz::openbmc_project::Common::Error;
+    namespace fs = std::experimental::filesystem;
+    namespace vpnor = openpower::virtual_pnor;
 
     if (context && !context->vpnor)
     {
-        context->vpnor = new vpnor_partition_table;
-        context->vpnor->table = new openpower::virtual_pnor::partition::Table(
-            std::move(dir), 1 << context->erase_size_shift,
-            context->flash_size);
+        fs::path dir(path);
+        try
+        {
+            context->vpnor = new vpnor_partition_table;
+            context->vpnor->table =
+                new openpower::virtual_pnor::partition::Table(
+                    std::move(dir), 1 << context->erase_size_shift,
+                    context->flash_size);
+        }
+        catch (vpnor::TocEntryError &e)
+        {
+            MSG_ERR("%s\n", e.what());
+            phosphor::logging::commit<err::InternalFailure>();
+            return -MBOX_R_SYSTEM_ERROR;
+        }
     }
+
+    return 0;
 }
 
 size_t vpnor_get_partition_table_size(const struct mbox_context *context)
@@ -60,7 +82,7 @@ vpnor_get_partition(const struct mbox_context *context, const size_t offset)
                : nullptr;
 }
 
-void vpnor_copy_bootloader_partition(const struct mbox_context *context)
+int vpnor_copy_bootloader_partition(const struct mbox_context *context)
 {
     // The hostboot bootloader has certain size/offset assumptions, so
     // we need a special partition table here.
@@ -76,26 +98,28 @@ void vpnor_copy_bootloader_partition(const struct mbox_context *context)
     constexpr size_t tocMaxSize = 0x8000;
     constexpr size_t tocStart = pnorSize - tocMaxSize - pageSize;
     constexpr auto blPartitionName = "HBB";
-    namespace fs = std::experimental::filesystem;
 
-    openpower::virtual_pnor::partition::Table blTable(
-        fs::path{PARTITION_FILES_RO_LOC}, eraseSize, pnorSize);
+    namespace err = sdbusplus::xyz::openbmc_project::Common::Error;
+    namespace fs = std::experimental::filesystem;
+    namespace vpnor = openpower::virtual_pnor;
 
-    vpnor_partition_table vtbl{};
-    vtbl.table = &blTable;
-    struct mbox_context local
-    {
-    };
-    local.vpnor = &vtbl;
-    local.block_size_shift = log_2(eraseSize);
-    memcpy(&local.paths, &context->paths, sizeof(local.paths));
-
-    size_t tocOffset = 0;
-    uint32_t tocSize = blTable.size() * eraseSize;
-    using namespace phosphor::logging;
-    using namespace sdbusplus::xyz::openbmc_project::Common::Error;
     try
     {
+        openpower::virtual_pnor::partition::Table blTable(
+            fs::path{PARTITION_FILES_RO_LOC}, eraseSize, pnorSize);
+
+        vpnor_partition_table vtbl{};
+        vtbl.table = &blTable;
+        struct mbox_context local
+        {
+        };
+        local.vpnor = &vtbl;
+        local.block_size_shift = log_2(eraseSize);
+        memcpy(&local.paths, &context->paths, sizeof(local.paths));
+
+        size_t tocOffset = 0;
+        uint32_t tocSize = blTable.size() * eraseSize;
+
         // Copy TOC
         copy_flash(&local, tocOffset,
                    static_cast<uint8_t *>(context->mem) + tocStart, tocSize);
@@ -106,10 +130,19 @@ void vpnor_copy_bootloader_partition(const struct mbox_context *context)
         copy_flash(&local, hbbOffset,
                    static_cast<uint8_t *>(context->mem) + hbbOffset, hbbSize);
     }
-    catch (InternalFailure &e)
+    catch (err::InternalFailure &e)
     {
-        commit<InternalFailure>();
+        phosphor::logging::commit<err::InternalFailure>();
+        return -MBOX_R_SYSTEM_ERROR;
     }
+    catch (vpnor::TocEntryError &e)
+    {
+        MSG_ERR("%s\n", e.what());
+        phosphor::logging::commit<err::InternalFailure>();
+        return -MBOX_R_SYSTEM_ERROR;
+    }
+
+    return 0;
 }
 
 void destroy_vpnor(struct mbox_context *context)
diff --git a/mboxd_pnor_partition_table.h b/mboxd_pnor_partition_table.h
index 738524b..122c933 100644
--- a/mboxd_pnor_partition_table.h
+++ b/mboxd_pnor_partition_table.h
@@ -27,8 +27,10 @@ extern "C" {
  *  This API should be called before calling any other APIs below. If a table
  *  already exists, this function will not do anything further. This function
  *  will not do anything if the context is NULL.
+ *
+ *  Returns 0 if the call succeeds, else a negative error code.
  */
-void init_vpnor(struct mbox_context *context);
+int init_vpnor(struct mbox_context *context);
 
 /** @brief Create a virtual PNOR partition table.
  *
@@ -37,10 +39,12 @@ void init_vpnor(struct mbox_context *context);
  *
  *  This API is same as above one but it reads the partition file from
  *  from the given location(path).
+ *
+ *  Returns 0 if the call succeeds, else a negative error code.
  */
 
-void vpnor_create_partition_table_from_path(struct mbox_context *context,
-                                            const char* path);
+int vpnor_create_partition_table_from_path(struct mbox_context *context,
+                                           const char* path);
 
 
 /** @brief Get partition table size, in blocks (1 block = 4KB)
@@ -82,8 +86,10 @@ const struct pnor_partition* vpnor_get_partition(
 /** @brief Copy bootloader partition (alongwith TOC) to LPC memory
  *
  *  @param[in] context - mbox context pointer
+ *
+ *  @returns 0 on success, negative error code on failure
  */
-void vpnor_copy_bootloader_partition(const struct mbox_context *context);
+int vpnor_copy_bootloader_partition(const struct mbox_context *context);
 
 /** @brief Destroy partition table, if it exists.
  *
diff --git a/pnor_partition_table.cpp b/pnor_partition_table.cpp
index b0d002e..983cddd 100644
--- a/pnor_partition_table.cpp
+++ b/pnor_partition_table.cpp
@@ -84,21 +84,31 @@ void Table::preparePartitions()
         pnor_partition& part = table.partitions[numParts];
         fs::path file;
 
-        if (!parseTocLine(line, blockSize, part))
+        // The ToC file presented in the vpnor squashfs looks like:
+        //
+        // version=IBM-witherspoon-ibm-OP9_v1.19_1.135
+        // extended_version=op-build-v1.19-571-g04f4690-dirty,buildroot-2017.11-5-g65679be,skiboot-v5.10-rc4,hostboot-4c46d66,linux-4.14.20-openpower1-p4a6b675,petitboot-v1.6.6-pe5aaec2,machine-xml-0fea226,occ-3286b6b,hostboot-binaries-3d1af8f,capp-ucode-p9-dd2-v3,sbe-99e2fe2
+        // partition00=part,0x00000000,0x00002000,00,READWRITE
+        // partition01=HBEL,0x00008000,0x0002c000,00,ECC,REPROVISION,CLEARECC,READWRITE
+        // ...
+        //
+        // As such we want to skip any lines that don't begin with 'partition'
+        if (std::string::npos == line.find("partition", 0))
         {
             continue;
         }
 
-        file = directory;
-        file /= part.data.name;
-        if (fs::exists(file))
-        {
-            ++numParts;
-        }
-        else
+        parseTocLine(line, blockSize, part);
+
+        file = directory / part.data.name;
+        if (!fs::exists(file))
         {
-            MSG_ERR("Partition file %s does not exist", file.c_str());
+            std::stringstream err;
+            err << "Partition file " << file.native() << " does not exist";
+            throw InvalidTocEntry(err.str());
         }
+
+        ++numParts;
     }
 }
 
@@ -264,7 +274,7 @@ static inline void writeNameAndId(pnor_partition& part, std::string&& name,
     part.data.id = std::stoul(id);
 }
 
-bool parseTocLine(const std::string& line, size_t blockSize,
+void parseTocLine(const std::string& line, size_t blockSize,
                   pnor_partition& part)
 {
     static constexpr auto ID_MATCH = 1;
@@ -285,7 +295,9 @@ bool parseTocLine(const std::string& line, size_t blockSize,
     std::smatch match;
     if (!std::regex_search(line, match, regex))
     {
-        return false;
+        std::stringstream err;
+        err << "Malformed partition description: " << line.c_str() << "\n";
+        throw MalformedTocEntry(err.str());
     }
 
     writeNameAndId(part, match[NAME_MATCH].str(), match[ID_MATCH].str());
@@ -300,8 +312,6 @@ bool parseTocLine(const std::string& line, size_t blockSize,
     unsigned long version = std::stoul(match[VERSION_MATCH].str(), nullptr, 16);
     writeUserdata(part, version << versionShift, match.suffix().str());
     part.checksum = details::checksum(part.data);
-
-    return true;
 }
 
 } // namespace virtual_pnor
diff --git a/pnor_partition_table.hpp b/pnor_partition_table.hpp
index 1105134..fae03a9 100644
--- a/pnor_partition_table.hpp
+++ b/pnor_partition_table.hpp
@@ -32,9 +32,9 @@ PartitionTable endianFixup(const PartitionTable& src);
  * @param[out] part - The partition object to populate with the information
  *                    parsed from the provided ToC line
  *
- * @returns True on success, false on failure.
+ * Throws: MalformedTocEntry, InvalidTocEntry
  */
-bool parseTocLine(const std::string& line, size_t blockSize,
+void parseTocLine(const std::string& line, size_t blockSize,
                   pnor_partition& part);
 
 namespace details
@@ -91,6 +91,8 @@ class Table
      *             open-power/hostboot/blob/master/src/usr/pnor/ffs.h for
      *             the PNOR FFS structure.
      *  @param[in] pnorSize - PNOR size, in bytes
+     *
+     * Throws MalformedTocEntry, InvalidTocEntry
      */
     Table(fs::path&& directory, size_t blockSize, size_t pnorSize);
 
@@ -151,6 +153,8 @@ class Table
 
   private:
     /** @brief Prepares a vector of PNOR partition structures.
+     *
+     * Throws: MalformedTocEntry, InvalidTocEntry
      */
     void preparePartitions();
 
@@ -199,7 +203,83 @@ class Table
     /** @brief PNOR size, in bytes */
     size_t pnorSize;
 };
-
 } // namespace partition
+
+/** @brief An exception type storing a reason string.
+ *
+ *  This looks a lot like how std::runtime_error might be implemented however
+ *  we want to avoid extending it, as exceptions extending ReasonedError have
+ *  an expectation of being handled (can be predicted and are inside the scope
+ *  of the program).
+ *
+ *  From std::runtime_error documentation[1]:
+ *
+ *  > Defines a type of object to be thrown as exception. It reports errors
+ *  > that are due to events beyond the scope of the program and can not be
+ *  > easily predicted.
+ *
+ *  [1] http://en.cppreference.com/w/cpp/error/runtime_error
+ *
+ *  We need to keep the inheritance hierarchy separate: This avoids the
+ *  introduction of code that overzealously catches std::runtime_error to
+ *  handle exceptions that would otherwise derive ReasonedError, and in the
+ *  process swallows genuine runtime failures.
+ */
+class ReasonedError : public std::exception
+{
+  public:
+    ReasonedError(const std::string&& what) : _what(what)
+    {
+    }
+    const char* what() const noexcept
+    {
+        return _what.c_str();
+    };
+
+  private:
+    const std::string _what;
+};
+
+/** @brief Base exception type for errors related to ToC entry parsing.
+ *
+ *  Callers of parseTocEntry() may not be concerned with the specifics and
+ *  rather just want to extract and log what().
+ */
+class TocEntryError : public ReasonedError
+{
+  public:
+    TocEntryError(const std::string&& reason) : ReasonedError(std::move(reason))
+    {
+    }
+};
+
+/** @brief The exception thrown on finding a syntax error in the ToC entry
+ *
+ *  If the syntax is wrong, or expected values are missing, the ToC entry is
+ *  malformed
+ */
+class MalformedTocEntry : public TocEntryError
+{
+  public:
+    MalformedTocEntry(const std::string&& reason) :
+        TocEntryError(std::move(reason))
+    {
+    }
+};
+
+/** @brief The exception thrown on finding a semantic error in the ToC entry
+ *
+ *  If the syntax of the ToC entry is correct but the semantics are broken,
+ *  then we have an invalid ToC entry.
+ */
+class InvalidTocEntry : public TocEntryError
+{
+  public:
+    InvalidTocEntry(const std::string&& reason) :
+        TocEntryError(std::move(reason))
+    {
+    }
+};
+
 } // namespace virtual_pnor
 } // namespace openpower