bmc: Simplify file handler
This greatly reduces the number of checks made against file operations
and fixes an issue with reading 0 length files.
Change-Id: I3d63ad0a3a49d6efbd047baadff3c0fe363b4174
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/bmc/file_handler.cpp b/bmc/file_handler.cpp
index dacca19..d2452ba 100644
--- a/bmc/file_handler.cpp
+++ b/bmc/file_handler.cpp
@@ -16,16 +16,14 @@
#include "file_handler.hpp"
-#include <cstdint>
#include <filesystem>
#include <ios>
-#include <memory>
-#include <string>
+#include <optional>
+#include <utility>
#include <vector>
namespace ipmi_flash
{
-namespace fs = std::filesystem;
bool FileHandler::open(const std::string& path, std::ios_base::openmode mode)
{
@@ -35,113 +33,55 @@
if (file.is_open())
{
- /* This wasn't properly closed somehow.
- * TODO: Throw an error or just reset the state?
- */
- return false;
+ return true;
}
-
file.open(filename, mode);
- if (!file.good()) /* on success goodbit is set */
- {
- /* TODO: Oh no! Care about this. */
- return false;
- }
-
- /* We were able to open the file for staging.
- * TODO: We'll need to do other stuff to eventually.
- */
- return true;
+ return file.good();
}
void FileHandler::close()
{
- if (file.is_open())
- {
- file.close();
- }
- return;
+ file.close();
}
bool FileHandler::write(std::uint32_t offset,
const std::vector<std::uint8_t>& data)
{
- if (!file.is_open())
- {
- return false;
- }
-
- /* We could track this, but if they write in a scattered method, this is
- * easier.
- */
- file.seekp(offset, std::ios_base::beg);
- if (!file.good())
- {
- /* the documentation wasn't super clear on fail vs bad in these cases,
- * so let's only be happy with goodness.
- */
- return false;
- }
-
+ file.seekp(offset);
file.write(reinterpret_cast<const char*>(data.data()), data.size());
- if (!file.good())
- {
- return false;
- }
-
- return true;
+ return file.good();
}
std::optional<std::vector<uint8_t>> FileHandler::read(std::uint32_t offset,
std::uint32_t size)
{
- if (!file.is_open())
+ uint32_t file_size = getSize();
+ if (offset > file_size)
{
return std::nullopt;
}
-
- /* determine size of file */
- file.seekg(0, std::ios_base::end);
- uint32_t filesize = file.tellg();
- uint32_t bytesToRead = size;
-
- /* make sure to not read past the end of file */
- if (offset + size > filesize)
- {
- bytesToRead = filesize - offset;
- }
-
- /* if no bytes can be read, fail */
- if (0 == bytesToRead)
- {
- return std::nullopt;
- }
-
- /* seek to offset then read */
+ std::vector<uint8_t> ret(std::min(file_size - offset, size));
file.seekg(offset);
- std::vector<uint8_t> fileData(bytesToRead);
- file.read(reinterpret_cast<char*>(fileData.data()), bytesToRead);
-
- /* if any sort of failure happened during all the seeks
- * and reads then fail the entire operation
- */
+ file.read(reinterpret_cast<char*>(ret.data()), ret.size());
if (!file.good())
{
return std::nullopt;
}
- return fileData;
+ return std::move(ret);
}
int FileHandler::getSize()
{
- try
+ std::error_code ec;
+ auto ret = std::filesystem::file_size(filename, ec);
+ if (ec)
{
- return static_cast<int>(fs::file_size(filename));
+ auto error = ec.message();
+ std::fprintf(stderr, "Failed to get filesize `%s`: %s\n",
+ filename.c_str(), error.c_str());
+ return 0;
}
- catch (const fs::filesystem_error& e)
- {}
-
- return 0;
+ return ret;
}
} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/test/file_handler_unittest.cpp b/bmc/firmware-handler/test/file_handler_unittest.cpp
index b579830..fbe774b 100644
--- a/bmc/firmware-handler/test/file_handler_unittest.cpp
+++ b/bmc/firmware-handler/test/file_handler_unittest.cpp
@@ -27,15 +27,15 @@
FileHandler handler(TESTPATH);
EXPECT_TRUE(handler.open(""));
-
- /* Calling open twice fails the second time. */
- EXPECT_FALSE(handler.open(""));
+ EXPECT_TRUE(handler.open(""));
}
TEST_F(FileHandlerOpenTest, VerifyOpenForReadFailsWithNoFile)
{
FileHandler handler(TESTPATH);
+ EXPECT_EQ(handler.getSize(), 0);
EXPECT_FALSE(handler.open("", std::ios::in));
+ EXPECT_EQ(handler.getSize(), 0);
}
TEST_F(FileHandlerOpenTest, VerifyOpenForReadSucceedsWithFile)
@@ -80,8 +80,10 @@
testPattern.size());
testfile.close();
FileHandler handler(TESTPATH);
+ EXPECT_EQ(handler.getSize(), testPattern.size());
EXPECT_TRUE(handler.open("", std::ios::in));
auto result = handler.read(0, 10);
+ EXPECT_EQ(handler.getSize(), testPattern.size());
ASSERT_TRUE(result);
EXPECT_EQ(result->size(), 10);
EXPECT_EQ(*result, testPattern);