firmware: tie implementation of session into write
To demonstrate how session will work, implement the write command.
Everything isn't wired up with open(), therefore this code itself will
only work in isolation.
This requires wiring up the open command to verify write will use the
handler we specify.
Change-Id: Icf5cfad4ddb531bc271642e24d505cbb9abf8f22
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index b51849d..2ee4155 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -4,6 +4,7 @@
#include <algorithm>
#include <cstdint>
+#include <cstring>
#include <memory>
#include <string>
#include <vector>
@@ -36,7 +37,11 @@
{
blobs.push_back(item.blobName);
}
- blobs.push_back(hashBlobID);
+
+ if (0 == std::count(blobs.begin(), blobs.end(), hashBlobID))
+ {
+ return nullptr;
+ }
std::uint16_t bitmask = 0;
for (const auto& item : transports)
@@ -169,7 +174,8 @@
*/
/* Check the flags for the transport mechanism: if none match we don't
- * support what they request. */
+ * support what they request.
+ */
if ((flags & bitmask) == 0)
{
return false;
@@ -179,15 +185,34 @@
if (path == activeImageBlobID)
{
/* 2a) are they opening the active image? this can only happen if they
- * already started one (due to canHandleBlob's behavior). */
+ * already started one (due to canHandleBlob's behavior).
+ */
}
else if (path == activeHashBlobID)
{
/* 2b) are they opening the active hash? this can only happen if they
- * already started one (due to canHandleBlob's behavior). */
+ * already started one (due to canHandleBlob's behavior).
+ */
}
- else if (path == hashBlobID)
+
+ /* How are they expecting to copy this data? */
+ auto d = std::find_if(
+ transports.begin(), transports.end(),
+ [&flags](const auto& iter) { return (iter.bitmask & flags); });
+ if (d == transports.end())
{
+ return false;
+ }
+
+ /* We found the transport handler they requested, no surprise since
+ * above we verify they selected at least one we wanted.
+ */
+ Session* curr;
+
+ if (path == hashBlobID)
+ {
+ curr = &activeHash;
+
/* 2c) are they opening the /flash/hash ? (to start the process) */
/* Add active hash blob_id to canHandle list. */
@@ -195,49 +220,38 @@
}
else
{
- /* How are they expecting to copy this data? */
- auto d = std::find_if(
- transports.begin(), transports.end(),
- [&flags](const auto& iter) { return (iter.bitmask & flags); });
- if (d == transports.end())
- {
- return false;
- }
-
- /* We found the transport handler they requested, no surprise since
- * above we verify they selected at least one we wanted.
- */
-
- /* 2d) are they opening the /flash/tarball ? (to start the UBI process)
- */
- /* 2e) are they opening the /flash/image ? (to start the process) */
- /* 2...) are they opening the /flash/... ? (to start the process) */
-
- auto h = std::find_if(
- handlers.begin(), handlers.end(),
- [&path](const auto& iter) { return (iter.blobName == path); });
- if (h == handlers.end())
- {
- return false;
- }
-
- /* Ok, so we found a handler that matched, so call open() */
- if (!h->handler->open(path))
- {
- return false;
- }
-
- /* open() succeeded. */
-
- /* TODO: Actually handle storing this information. */
+ curr = &activeImage;
/* add active image blob_id to canHandle list. */
blobIDs.push_back(activeImageBlobID);
-
- return true;
}
- return false;
+ /* 2d) are they opening the /flash/tarball ? (to start the UBI process)
+ */
+ /* 2e) are they opening the /flash/image ? (to start the process) */
+ /* 2...) are they opening the /flash/... ? (to start the process) */
+
+ auto h = std::find_if(
+ handlers.begin(), handlers.end(),
+ [&path](const auto& iter) { return (iter.blobName == path); });
+ if (h == handlers.end())
+ {
+ return false;
+ }
+
+ /* Ok, so we found a handler that matched, so call open() */
+ if (!h->handler->open(path))
+ {
+ return false;
+ }
+
+ curr->flags = flags;
+ curr->dataHandler = d->handler;
+ curr->imageHandler = h->handler;
+
+ lookup[session] = curr;
+
+ return true;
}
std::vector<uint8_t> FirmwareBlobHandler::read(uint16_t session,
@@ -251,15 +265,45 @@
return {};
}
+/**
+ * The write command really just grabs the data from wherever it is and sends it
+ * to the image handler. It's the image handler's responsibility to deal with
+ * the data provided.
+ */
bool FirmwareBlobHandler::write(uint16_t session, uint32_t offset,
const std::vector<uint8_t>& data)
{
- /*
- * This will do whatever behavior is expected by mechanism - likely will
- * just call the specific write handler.
- */
- return false;
+ auto item = lookup.find(session);
+ if (item == lookup.end())
+ {
+ return false;
+ }
+
+ std::vector<std::uint8_t> bytes;
+
+ if (item->second->flags & FirmwareUpdateFlags::ipmi)
+ {
+ bytes = data;
+ }
+ else
+ {
+ /* little endian required per design, and so on, but TODO: do endianness
+ * with boost.
+ */
+ struct ExtChunkHdr header;
+
+ if (data.size() != sizeof(header))
+ {
+ return false;
+ }
+
+ std::memcpy(&header, data.data(), data.size());
+ bytes = item->second->dataHandler->copyFrom(header.length);
+ }
+
+ return item->second->imageHandler->write(offset, bytes);
}
+
bool FirmwareBlobHandler::writeMeta(uint16_t session, uint32_t offset,
const std::vector<uint8_t>& data)
{
diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index 0c3092b..4dcbfb8 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -25,8 +25,16 @@
/** Pointer to the correct image handler interface. (nullptr on hash
* blob_id) */
ImageHandlerInterface* imageHandler;
+
+ /** The flags used to open the session. */
+ std::uint16_t flags;
};
+struct ExtChunkHdr
+{
+ std::uint32_t length; /* Length of the data queued (little endian). */
+} __attribute__((packed));
+
/**
* Register only one firmware blob handler that will manage all sessions.
*/
diff --git a/main.cpp b/main.cpp
index 4a5b8a6..44b5836 100644
--- a/main.cpp
+++ b/main.cpp
@@ -1,6 +1,7 @@
#include "config.h"
#include "firmware_handler.hpp"
+#include "hash_handler.hpp"
#include "image_handler.hpp"
#include "lpc_handler.hpp"
#include "pci_handler.hpp"
@@ -17,11 +18,13 @@
namespace
{
+HashFileHandler hashHandler;
StaticLayoutHandler staticLayoutHandler;
LpcDataHandler lpcDataHandler;
PciDataHandler pciDataHandler;
std::vector<HandlerPack> supportedFirmware = {
+ {FirmwareBlobHandler::hashBlobID, &hashHandler},
#ifdef ENABLE_STATIC_LAYOUT
{"/flash/image", &staticLayoutHandler},
#endif
diff --git a/test/Makefile.am b/test/Makefile.am
index 7ea9cd6..243e733 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,7 +13,8 @@
firmware_handler_unittest \
firmware_stat_unittest \
firmware_canhandle_unittest \
- firmware_open_unittest
+ firmware_open_unittest \
+ firmware_write_unittest
TESTS = $(check_PROGRAMS)
@@ -28,3 +29,6 @@
firmware_open_unittest_SOURCES = firmware_open_unittest.cpp
firmware_open_unittest_LDADD = $(top_builddir)/firmware_handler.o
+
+firmware_write_unittest_SOURCES = firmware_write_unittest.cpp
+firmware_write_unittest_LDADD = $(top_builddir)/firmware_handler.o
diff --git a/test/firmware_canhandle_unittest.cpp b/test/firmware_canhandle_unittest.cpp
index 600be4b..44223d4 100644
--- a/test/firmware_canhandle_unittest.cpp
+++ b/test/firmware_canhandle_unittest.cpp
@@ -23,6 +23,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
{"bcdf", &imageMock},
};
diff --git a/test/firmware_handler_unittest.cpp b/test/firmware_handler_unittest.cpp
index 6a1ffe6..e6998c1 100644
--- a/test/firmware_handler_unittest.cpp
+++ b/test/firmware_handler_unittest.cpp
@@ -24,14 +24,16 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, {});
EXPECT_EQ(handler, nullptr);
}
-TEST(FirmwareHandlerTest, CreateEmptyListVerifyHasHash)
+TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
{
+ /* This works fine only if you also pass in the hash handler. */
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
@@ -42,6 +44,11 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+ EXPECT_EQ(handler, nullptr);
+
+ blobs.push_back({FirmwareBlobHandler::hashBlobID, &imageMock});
+
+ handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
auto result = handler->getBlobIds();
EXPECT_EQ(2, result.size());
EXPECT_EQ(2, std::count(result.begin(), result.end(), "asdf") +
diff --git a/test/firmware_open_unittest.cpp b/test/firmware_open_unittest.cpp
index bc6ece2..2b1a330 100644
--- a/test/firmware_open_unittest.cpp
+++ b/test/firmware_open_unittest.cpp
@@ -20,6 +20,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
@@ -43,6 +44,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
@@ -65,6 +67,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
@@ -84,6 +87,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
@@ -103,6 +107,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
diff --git a/test/firmware_stat_unittest.cpp b/test/firmware_stat_unittest.cpp
index e4b09bc..ad43b60 100644
--- a/test/firmware_stat_unittest.cpp
+++ b/test/firmware_stat_unittest.cpp
@@ -20,6 +20,7 @@
ImageHandlerMock imageMock;
std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock},
{"asdf", &imageMock},
};
std::vector<DataHandlerPack> data = {
diff --git a/test/firmware_write_unittest.cpp b/test/firmware_write_unittest.cpp
new file mode 100644
index 0000000..bac2d39
--- /dev/null
+++ b/test/firmware_write_unittest.cpp
@@ -0,0 +1,121 @@
+#include "data_mock.hpp"
+#include "firmware_handler.hpp"
+#include "image_mock.hpp"
+
+#include <cstdint>
+#include <cstring>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace blobs
+{
+using ::testing::ElementsAreArray;
+using ::testing::Eq;
+using ::testing::Return;
+
+TEST(FirmwareHandlerWriteTest, DataTypeIpmiWriteSuccess)
+{
+ /* Verify if data type ipmi, it calls write with the bytes. */
+
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::FirmwareUpdateFlags::ipmi, nullptr},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(
+ 0, OpenFlags::write | FirmwareBlobHandler::FirmwareUpdateFlags::ipmi,
+ "asdf"));
+
+ std::vector<std::uint8_t> bytes = {0xaa, 0x55};
+
+ EXPECT_CALL(imageMock2, write(0, Eq(bytes))).WillOnce(Return(true));
+ EXPECT_TRUE(handler->write(0, 0, bytes));
+}
+
+TEST(FirmwareHandlerWriteTest, DataTypeNonIpmiWriteSuccess)
+{
+ /* Verify if data type non-ipmi, it calls write with the length. */
+
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ DataHandlerMock dataMock;
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::FirmwareUpdateFlags::ipmi, nullptr},
+ {FirmwareBlobHandler::FirmwareUpdateFlags::lpc, &dataMock},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(
+ 0, OpenFlags::write | FirmwareBlobHandler::FirmwareUpdateFlags::lpc,
+ "asdf"));
+
+ struct ExtChunkHdr request;
+ request.length = 4; /* number of bytes to read. */
+ std::vector<std::uint8_t> ipmiRequest;
+ ipmiRequest.resize(sizeof(request));
+ std::memcpy(ipmiRequest.data(), &request, sizeof(request));
+
+ std::vector<std::uint8_t> bytes = {0x01, 0x02, 0x03, 0x04};
+
+ EXPECT_CALL(dataMock, copyFrom(request.length)).WillOnce(Return(bytes));
+ EXPECT_CALL(imageMock2, write(0, Eq(bytes))).WillOnce(Return(true));
+ EXPECT_TRUE(handler->write(0, 0, ipmiRequest));
+}
+
+TEST(FirmwareHandlerWriteTest, DataTypeNonIpmiWriteFailsBadRequest)
+{
+ /* Verify the data type non-ipmi, if the request's structure doesn't match,
+ * return failure. */
+
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ DataHandlerMock dataMock;
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::FirmwareUpdateFlags::ipmi, nullptr},
+ {FirmwareBlobHandler::FirmwareUpdateFlags::lpc, &dataMock},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(
+ 0, OpenFlags::write | FirmwareBlobHandler::FirmwareUpdateFlags::lpc,
+ "asdf"));
+
+ struct ExtChunkHdr request;
+ request.length = 4; /* number of bytes to read. */
+
+ std::vector<std::uint8_t> ipmiRequest;
+ ipmiRequest.resize(sizeof(request));
+ std::memcpy(ipmiRequest.data(), &request, sizeof(request));
+ ipmiRequest.push_back(1);
+
+ /* ipmiRequest is too large by one byte. */
+ EXPECT_FALSE(handler->write(0, 0, ipmiRequest));
+}
+
+} // namespace blobs