From ad343101c59beb0c2b0c1033f031ff76ff2237d7 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 28 Feb 2018 00:35:50 +1030 Subject: pnor_partition: Refactor to enforce stronger boundaries for abstractions The RORequest and RWRequest classes did not provide a clear abstraction over the operation of populating a window or partition associated with a CREATE_{READ,WRITE}_WINDOW request. The role of the classes was to find the partition for the provided offset, locate and then open its backing file. However, the file-descriptor for the backing file was exposed outside of the class, as was the FFS partition struct, both of which were managed _internal_ to the class. Thus the classes provided no encapsulation of state and awkwardly split the tasks of managing and utilising the resources between the callee and caller. This commit inverts the behaviour in a fulfil() method handles the mechanics of locating, opening, manipulating and closing the backing file, requiring nothing of the caller. The pnor_partition reference is managed entirely inside the Request class, derived from the offset passed to the constructor. Unifying the mechanics into fulfil() results in a decent reduction in lines of code at the expense of some cyclomatic complexity. fulfil() is publicly exposed via read() and write() wrappers on the class, and the RORequest and RWRequest classes are removed as a result. Change-Id: Ie587ed31f1a6db97bcb490dbcc2e27ece0b1f82c Signed-off-by: Andrew Jeffery --- pnor_partition.hpp | 120 +++++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 63 deletions(-) (limited to 'pnor_partition.hpp') diff --git a/pnor_partition.hpp b/pnor_partition.hpp index a11a921..87bae3b 100644 --- a/pnor_partition.hpp +++ b/pnor_partition.hpp @@ -2,7 +2,12 @@ /* Copyright (C) 2018 IBM Corp. */ #pragma once +extern "C" { +#include "mbox.h" +}; + #include "mboxd_pnor_partition_table.h" +#include "pnor_partition_table.hpp" #include #include #include @@ -68,51 +73,46 @@ enum class ReturnCode : uint8_t class Request { public: - Request() = default; + /** @brief Construct a flash access request + * + * @param[in] ctx - The mbox context used to process the request + * @param[in] offset - The absolute offset into the flash device as + * provided by the mbox message associated with the + * request + * + * The class does not take ownership of the ctx pointer. The lifetime of + * the ctx pointer must strictly exceed the lifetime of the class + * instance. + */ + Request(struct mbox_context* ctx, size_t offset) : + ctx(ctx), partition(ctx->vpnor->table->partition(offset)), + base(partition.data.base << ctx->block_size_shift), + offset(offset - base) + { + } Request(const Request&) = delete; Request& operator=(const Request&) = delete; Request(Request&&) = default; Request& operator=(Request&&) = default; ~Request() = default; - openpower::file::Descriptor fd; + ssize_t read(void* dst, size_t len) + { + return fulfil(dst, len, O_RDONLY); + } - protected: - /** @brief opens the partition file - * - * @param[in] filePath - Absolute file path. - * @param[in] mode - File open mode. - */ - ReturnCode open(const std::string& filePath, int mode); + ssize_t write(void* dst, size_t len) + { + return fulfil(dst, len, O_RDWR); + } - /** @brief returns the partition file path associated with the offset. + private: + /** @brief Returns the partition file path associated with the offset. * - * @param[in] context - The mbox context pointer. - * @param[in] offset - The pnor offset(bytes). - */ - - std::string getPartitionFilePath(struct mbox_context* context, - uint32_t offset); - - const pnor_partition* partition = nullptr; -}; - -/** @class RORequest - * @brief Represent the read request of the partition. - * Stores the partition meta data. - */ -class RORequest : public Request -{ - public: - RORequest() = default; - RORequest(const RORequest&) = delete; - RORequest& operator=(const RORequest&) = delete; - RORequest(RORequest&&) = default; - RORequest& operator=(RORequest&&) = default; - ~RORequest(){}; - - /** @brief opens the partition file associated with the offset - * in read only mode and gets the partition details. + * The search strategy for the partition file depends on the value of the + * flags parameter. + * + * For the O_RDONLY case: * * 1. Depending on the partition type,tries to open the file * from the associated partition(RW/PRSV/RO). @@ -122,29 +122,7 @@ class RORequest : public Request * 1b. if the file not found in the read only partition then * throw exception. * - * @param[in] context - The mbox context pointer. - * @param[in] offset - The pnor offset(bytes). - */ - const pnor_partition* getPartitionInfo(struct mbox_context* context, - uint32_t offset); -}; - -/** @class RWRequest - * @brief Represent the write request of the partition. - * Stores the partition meta data. - */ -class RWRequest : public Request -{ - public: - RWRequest() = default; - RWRequest(const RWRequest&) = delete; - RWRequest& operator=(const RWRequest&) = delete; - RWRequest(RWRequest&&) = default; - RWRequest& operator=(RWRequest&&) = default; - ~RWRequest(){}; - - /** @brief opens the partition file associated with the offset - * in write mode and gets the parttition details. + * For the O_RDWR case: * * 1. Depending on the partition type tries to open the file * from the associated partition. @@ -152,13 +130,29 @@ class RWRequest : public Request * then copy the file from the read only partition to the (RW/PRSV) * partition depending on the partition type. * 1b. if the file not found in the read only partition then throw - * exception. + * exception. + * + * @param[in] flags - The flags that will be used to open the file. Must + * be one of O_RDONLY or O_RDWR. + * + * Post-condition: The file described by the returned path exists + * + * Throws: std::filesystem_error, std::bad_alloc + */ + std::experimental::filesystem::path getPartitionFilePath(int flags); + + /** @brief Fill dst with the content of the partition relative to offset. * - * @param[in] context - The mbox context pointer. * @param[in] offset - The pnor offset(bytes). + * @param[out] dst - The buffer to fill with partition data + * @param[in] len - The length of the destination buffer */ - const pnor_partition* getPartitionInfo(struct mbox_context* context, - uint32_t offset); + ssize_t fulfil(void* dst, size_t len, int flags); + + struct mbox_context* ctx; + const pnor_partition& partition; + size_t base; + size_t offset; }; } // namespace virtual_pnor -- cgit v1.2.1