extend file_handler to support reads
Problem: the upcomming version handler will need to read from files.
Currently file hander (image handler) does not support reads.
Solution: Add read support by providing an optional mode parameter.
Tests added:
FileHanderTest added to
- Test out open for reads
- Test out reading out bytes and verifying size and content
- Test out trying to read beyond EOF
- Test out offset reads that go beyond EOF
Tested:
Existing unit tests pass
New unit tests for reading all pass
Signed-off-by: Jason Ling <jasonling@google.com>
Change-Id: Ie416a6b4b452d8d04fa158bd55989d07a891896f
diff --git a/bmc/firmware-handler/test/file_handler_unittest.cpp b/bmc/firmware-handler/test/file_handler_unittest.cpp
index ab12125..b579830 100644
--- a/bmc/firmware-handler/test/file_handler_unittest.cpp
+++ b/bmc/firmware-handler/test/file_handler_unittest.cpp
@@ -21,7 +21,7 @@
}
};
-TEST_F(FileHandlerOpenTest, VerifyItIsHappy)
+TEST_F(FileHandlerOpenTest, VerifyDefaultOpenIsHappy)
{
/* Opening a fail may create it? */
@@ -32,6 +32,21 @@
EXPECT_FALSE(handler.open(""));
}
+TEST_F(FileHandlerOpenTest, VerifyOpenForReadFailsWithNoFile)
+{
+ FileHandler handler(TESTPATH);
+ EXPECT_FALSE(handler.open("", std::ios::in));
+}
+
+TEST_F(FileHandlerOpenTest, VerifyOpenForReadSucceedsWithFile)
+{
+ std::ofstream testfile;
+ testfile.open(TESTPATH, std::ios::out);
+ testfile << "Hello world";
+ FileHandler handler(TESTPATH);
+ EXPECT_TRUE(handler.open("", std::ios::in));
+}
+
TEST_F(FileHandlerOpenTest, VerifyWriteDataWrites)
{
/* Verify writing bytes writes them... flushing data can be an issue here,
@@ -55,4 +70,55 @@
/* annoyingly the memcmp was failing... but it's the same data. */
}
+TEST_F(FileHandlerOpenTest, VerifySimpleRead)
+{
+ std::ofstream testfile;
+ testfile.open(TESTPATH, std::ios::out);
+ std::vector<std::uint8_t> testPattern = {0x0, 0x1, 0x2, 0x3, 0x4,
+ 0x5, 0x6, 0x7, 0x8, 0x9};
+ testfile.write(reinterpret_cast<const char*>(testPattern.data()),
+ testPattern.size());
+ testfile.close();
+ FileHandler handler(TESTPATH);
+ EXPECT_TRUE(handler.open("", std::ios::in));
+ auto result = handler.read(0, 10);
+ ASSERT_TRUE(result);
+ EXPECT_EQ(result->size(), 10);
+ EXPECT_EQ(*result, testPattern);
+}
+
+TEST_F(FileHandlerOpenTest, VerifyTruncatedAndOffsetReads)
+{
+ std::ofstream testfile;
+ testfile.open(TESTPATH, std::ios::out);
+ std::vector<std::uint8_t> testPattern = {0x0, 0x1, 0x2, 0x3, 0x4,
+ 0x5, 0x6, 0x7, 0x8, 0x9};
+ std::vector<std::uint8_t> expectedResult(testPattern.begin() + 3,
+ testPattern.end());
+
+ testfile.write(reinterpret_cast<const char*>(testPattern.data()),
+ testPattern.size());
+ testfile.close();
+ FileHandler handler(TESTPATH);
+ EXPECT_TRUE(handler.open("", std::ios::in));
+ auto result = handler.read(3, 10);
+ ASSERT_TRUE(result);
+ EXPECT_EQ(*result, expectedResult);
+}
+
+TEST_F(FileHandlerOpenTest, VerifyBadOffsetReadsFail)
+{
+ std::ofstream testfile;
+ testfile.open(TESTPATH, std::ios::out);
+ std::vector<std::uint8_t> testPattern = {0x0, 0x1, 0x2, 0x3, 0x4,
+ 0x5, 0x6, 0x7, 0x8, 0x9};
+ testfile.write(reinterpret_cast<const char*>(testPattern.data()),
+ testPattern.size());
+ testfile.close();
+ FileHandler handler(TESTPATH);
+ EXPECT_TRUE(handler.open("", std::ios::in));
+ auto result = handler.read(11, 10);
+ EXPECT_FALSE(result);
+}
+
} // namespace ipmi_flash
diff --git a/bmc/firmware-handler/test/firmware_close_unittest.cpp b/bmc/firmware-handler/test/firmware_close_unittest.cpp
index 6d1fe55..2a722b6 100644
--- a/bmc/firmware-handler/test/firmware_close_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_close_unittest.cpp
@@ -28,7 +28,8 @@
* everything looks right.
*/
EXPECT_CALL(*dataMock, open()).WillOnce(Return(true));
- EXPECT_CALL(*hashImageMock, open(StrEq(hashBlobId))).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(StrEq(hashBlobId), std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::lpc,
@@ -55,7 +56,8 @@
/* Boring test where you open a blob_id using ipmi, so there's no data
* handler, and it's closed and everything looks right.
*/
- EXPECT_CALL(*hashImageMock, open(StrEq(hashBlobId))).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(StrEq(hashBlobId), std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi,
diff --git a/bmc/firmware-handler/test/firmware_commit_unittest.cpp b/bmc/firmware-handler/test/firmware_commit_unittest.cpp
index cb50b8e..d564de6 100644
--- a/bmc/firmware-handler/test/firmware_commit_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_commit_unittest.cpp
@@ -48,7 +48,8 @@
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
std::move(blobs), std::move(data), CreateActionMap("asdf"));
- EXPECT_CALL(*imageMock2, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock2, open("asdf", std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi, "asdf"));
@@ -64,7 +65,8 @@
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
std::move(blobs), std::move(data), CreateActionMap("asdf"));
- EXPECT_CALL(*imageMock1, open(StrEq(hashBlobId))).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock1, open(StrEq(hashBlobId), std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi,
diff --git a/bmc/firmware-handler/test/firmware_multiplebundle_unittest.cpp b/bmc/firmware-handler/test/firmware_multiplebundle_unittest.cpp
index 20c63a9..58f07c7 100644
--- a/bmc/firmware-handler/test/firmware_multiplebundle_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_multiplebundle_unittest.cpp
@@ -118,7 +118,7 @@
/* You can only have one file open at a time, and the first firmware file
* you open locks it down
*/
- EXPECT_CALL(*staticImageMock, open(staticLayoutBlobId))
+ EXPECT_CALL(*staticImageMock, open(staticLayoutBlobId, std::ios::out))
.WillOnce(Return(true));
EXPECT_CALL(*bmcPrepareMockPtr, trigger()).WillOnce(Return(true));
@@ -130,14 +130,15 @@
expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
- EXPECT_CALL(*biosImageMock, open(biosBlobId)).Times(0);
+ EXPECT_CALL(*biosImageMock, open(biosBlobId, std::ios::out)).Times(0);
EXPECT_FALSE(handler->open(session, flags, biosBlobId));
}
TEST_F(IpmiOnlyTwoFirmwaresTest, OpeningHashBeforeBiosSucceeds)
{
/* Opening the hash blob does nothing special in this regard. */
- EXPECT_CALL(*hashImageMock, open(hashBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(hashBlobId, std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, hashBlobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
@@ -148,7 +149,8 @@
expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
ASSERT_FALSE(handler->canHandleBlob(verifyBlobId));
- EXPECT_CALL(*biosImageMock, open(biosBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*biosImageMock, open(biosBlobId, std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, biosBlobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
diff --git a/bmc/firmware-handler/test/firmware_sessionstat_unittest.cpp b/bmc/firmware-handler/test/firmware_sessionstat_unittest.cpp
index 0736293..3753118 100644
--- a/bmc/firmware-handler/test/firmware_sessionstat_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_sessionstat_unittest.cpp
@@ -25,7 +25,7 @@
/* Verifying running stat if the type of data session is IPMI returns no
* metadata.
*/
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi, "asdf"));
@@ -48,7 +48,7 @@
* simply implementing read().
*/
EXPECT_CALL(*dataMock, open()).WillOnce(Return(true));
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::lpc, "asdf"));
diff --git a/bmc/firmware-handler/test/firmware_state_notyetstarted_tarball_unittest.cpp b/bmc/firmware-handler/test/firmware_state_notyetstarted_tarball_unittest.cpp
index e666395..6a39d9d 100644
--- a/bmc/firmware-handler/test/firmware_state_notyetstarted_tarball_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_state_notyetstarted_tarball_unittest.cpp
@@ -65,11 +65,13 @@
{
if (blobId == hashBlobId)
{
- EXPECT_CALL(*hashImageMock, open(blobId)).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(blobId, std::ios::out))
+ .WillOnce(Return(true));
}
else
{
- EXPECT_CALL(*imageMock, open(blobId)).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open(blobId, std::ios::out))
+ .WillOnce(Return(true));
}
EXPECT_TRUE(handler->open(session, flags, blobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
diff --git a/bmc/firmware-handler/test/firmware_state_notyetstarted_unittest.cpp b/bmc/firmware-handler/test/firmware_state_notyetstarted_unittest.cpp
index 970ae2f..0193e91 100644
--- a/bmc/firmware-handler/test/firmware_state_notyetstarted_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_state_notyetstarted_unittest.cpp
@@ -88,7 +88,8 @@
*/
TEST_F(FirmwareHandlerNotYetStartedTest, OpenStaticImageFileVerifyStateChange)
{
- EXPECT_CALL(*imageMock2, open(staticLayoutBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock2, open(staticLayoutBlobId, std::ios::out))
+ .WillOnce(Return(true));
EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
@@ -100,7 +101,8 @@
TEST_F(FirmwareHandlerNotYetStartedTest, OpenHashFileVerifyStateChange)
{
- EXPECT_CALL(*hashImageMock, open(hashBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(hashBlobId, std::ios::out))
+ .WillOnce(Return(true));
/* Opening the hash blob id doesn't trigger a preparation, only a firmware
* blob.
*/
diff --git a/bmc/firmware-handler/test/firmware_state_verificationpending_unittest.cpp b/bmc/firmware-handler/test/firmware_state_verificationpending_unittest.cpp
index dc6f3d3..6188890 100644
--- a/bmc/firmware-handler/test/firmware_state_verificationpending_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_state_verificationpending_unittest.cpp
@@ -240,7 +240,8 @@
/* Verifies it isn't triggered again. */
EXPECT_CALL(*prepareMockPtr, trigger()).Times(0);
- EXPECT_CALL(*imageMock2, open(staticLayoutBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock2, open(staticLayoutBlobId, std::ios::out))
+ .WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
diff --git a/bmc/firmware-handler/test/firmware_unittest.hpp b/bmc/firmware-handler/test/firmware_unittest.hpp
index e9db343..da2e8f7 100644
--- a/bmc/firmware-handler/test/firmware_unittest.hpp
+++ b/bmc/firmware-handler/test/firmware_unittest.hpp
@@ -76,11 +76,13 @@
{
if (blobId == hashBlobId)
{
- EXPECT_CALL(*hashImageMock, open(blobId)).WillOnce(Return(true));
+ EXPECT_CALL(*hashImageMock, open(blobId, std::ios::out))
+ .WillOnce(Return(true));
}
else
{
- EXPECT_CALL(*imageMock2, open(blobId)).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock2, open(blobId, std::ios::out))
+ .WillOnce(Return(true));
}
if (blobId != hashBlobId)
diff --git a/bmc/firmware-handler/test/firmware_write_unittest.cpp b/bmc/firmware-handler/test/firmware_write_unittest.cpp
index e9b70ba..27899db 100644
--- a/bmc/firmware-handler/test/firmware_write_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_write_unittest.cpp
@@ -29,7 +29,7 @@
TEST_F(FirmwareHandlerWriteTestIpmiOnly, DataTypeIpmiWriteSuccess)
{
/* Verify if data type ipmi, it calls write with the bytes. */
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi, "asdf"));
@@ -44,7 +44,7 @@
{
/* Verify if data type non-ipmi, it calls write with the length. */
EXPECT_CALL(*dataMock, open()).WillOnce(Return(true));
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::lpc, "asdf"));
@@ -67,7 +67,7 @@
/* Verify the data type non-ipmi, if the request's structure doesn't match,
* return failure. */
EXPECT_CALL(*dataMock, open()).WillOnce(Return(true));
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::lpc, "asdf"));
diff --git a/bmc/firmware-handler/test/firmware_writemeta_unittest.cpp b/bmc/firmware-handler/test/firmware_writemeta_unittest.cpp
index 512754a..3d86c0c 100644
--- a/bmc/firmware-handler/test/firmware_writemeta_unittest.cpp
+++ b/bmc/firmware-handler/test/firmware_writemeta_unittest.cpp
@@ -23,7 +23,7 @@
TEST_F(FirmwareHandlerWriteMetaTest, WriteConfigParametersFailIfOverIPMI)
{
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi, "asdf"));
@@ -36,7 +36,7 @@
TEST_F(FirmwareHandlerWriteMetaTest, WriteConfigParametersPassedThrough)
{
EXPECT_CALL(*dataMock, open()).WillOnce(Return(true));
- EXPECT_CALL(*imageMock, open("asdf")).WillOnce(Return(true));
+ EXPECT_CALL(*imageMock, open("asdf", std::ios::out)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(
0, blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::lpc, "asdf"));
diff --git a/bmc/firmware-handler/test/image_mock.hpp b/bmc/firmware-handler/test/image_mock.hpp
index fb10c39..2a967ac 100644
--- a/bmc/firmware-handler/test/image_mock.hpp
+++ b/bmc/firmware-handler/test/image_mock.hpp
@@ -11,10 +11,11 @@
{
public:
virtual ~ImageHandlerMock() = default;
-
- MOCK_METHOD1(open, bool(const std::string&));
+ MOCK_METHOD2(open, bool(const std::string&, std::ios_base::openmode));
MOCK_METHOD0(close, void());
MOCK_METHOD2(write, bool(std::uint32_t, const std::vector<std::uint8_t>&));
+ MOCK_METHOD2(read, std::optional<std::vector<std::uint8_t>>(std::uint32_t,
+ std::uint32_t));
MOCK_METHOD0(getSize, int());
};