Add unit test for ReadFileIntoMemory and WriteFileFromMemory
The code is refactored to support mocking the transferDataHost API.
Unit testcases are added for ReadFileIntoMemory and WriteFileFromMemory
responder implementations.
Signed-off-by: Tom Joseph <tomjoseph@in.ibm.com>
Change-Id: Iab9966a7aea602cc2abcdf386f077001368e66d9
diff --git a/.gitignore b/.gitignore
index ba334df..c3d3d00 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,4 +60,6 @@
/test-suite.log
/test/*.gcda
/test/*.gcno
-/test/libpldm_fileio_test
+/test/libpldmoem_fileio_test
+/test/libpldmoemresponder_fileio_test
+
diff --git a/libpldmresponder/Makefile.am b/libpldmresponder/Makefile.am
index 597cbd3..c043c8b 100644
--- a/libpldmresponder/Makefile.am
+++ b/libpldmresponder/Makefile.am
@@ -7,4 +7,4 @@
../libpldm/libpldmoem.la
libpldmoemresponder_la_LDFLAGS = \
-version-info 1:0:0 -shared \
- -lstdc++fs
+ -lstdc++fs
diff --git a/libpldmresponder/file_io.cpp b/libpldmresponder/file_io.cpp
index ed043c6..06d683b 100644
--- a/libpldmresponder/file_io.cpp
+++ b/libpldmresponder/file_io.cpp
@@ -40,8 +40,8 @@
constexpr auto xdmaDev = "/dev/xdma";
-int transferDataHost(const fs::path& path, uint32_t offset, uint32_t length,
- uint64_t address, bool upstream)
+int DMA::transferDataHost(const fs::path& path, uint32_t offset,
+ uint32_t length, uint64_t address, bool upstream)
{
static const size_t pageSize = getpagesize();
uint32_t numPages = length / pageSize;
@@ -126,38 +126,6 @@
} // namespace dma
-void transferAll(uint8_t command, fs::path& path, uint32_t offset,
- uint32_t length, uint64_t address, bool upstream,
- pldm_msg* response)
-{
- uint32_t origLength = length;
-
- while (length > dma::maxSize)
- {
- auto rc = dma::transferDataHost(path, offset, dma::maxSize, address,
- upstream);
- if (rc < 0)
- {
- encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, response);
- return;
- }
-
- offset += dma::maxSize;
- length -= dma::maxSize;
- address += dma::maxSize;
- }
-
- auto rc = dma::transferDataHost(path, offset, length, address, upstream);
- if (rc < 0)
- {
- encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, response);
- return;
- }
-
- encode_rw_file_memory_resp(0, command, PLDM_SUCCESS, origLength, response);
- return;
-}
-
void readFileIntoMemory(const uint8_t* request, size_t payloadLength,
pldm_msg* response)
{
@@ -209,8 +177,10 @@
return;
}
- transferAll(PLDM_READ_FILE_INTO_MEMORY, path, offset, length, address, true,
- response);
+ using namespace dma;
+ DMA intf;
+ transferAll<DMA>(&intf, PLDM_READ_FILE_INTO_MEMORY, path, offset, length,
+ address, true, response);
}
void writeFileFromMemory(const uint8_t* request, size_t payloadLength,
@@ -259,8 +229,10 @@
return;
}
- transferAll(PLDM_WRITE_FILE_FROM_MEMORY, path, offset, length, address,
- false, response);
+ using namespace dma;
+ DMA intf;
+ transferAll<DMA>(&intf, PLDM_WRITE_FILE_FROM_MEMORY, path, offset, length,
+ address, false, response);
}
} // namespace responder
diff --git a/libpldmresponder/file_io.hpp b/libpldmresponder/file_io.hpp
index ae475b9..ea3a932 100644
--- a/libpldmresponder/file_io.hpp
+++ b/libpldmresponder/file_io.hpp
@@ -51,8 +51,6 @@
} // namespace utils
-namespace fs = std::filesystem;
-
namespace dma
{
@@ -62,20 +60,34 @@
// 16MB - 4096B (16773120 bytes) is the maximum data size of DMA transfer
constexpr size_t maxSize = (16 * 1024 * 1024) - 4096;
-/** @brief API to transfer data between BMC and host using DMA
+namespace fs = std::filesystem;
+
+/**
+ * @class DMA
*
- * @param[in] path - pathname of the file to transfer data from or to
- * @param[in] offset - offset in the file
- * @param[in] length - length of the data to transfer
- * @param[in] address - DMA address on the host
- * @param[in] upstream - indicates directon of the transfer; true indicates
- * transfer to the host
+ * Expose API to initiate transfer of data by DMA
*
- * @return - returns 0 on success, negative errno on failure
+ * This class only exposes the public API transferDataHost to transfer data
+ * between BMC and host using DMA. This allows for mocking the transferDataHost
+ * for unit testing purposes.
*/
-int transferDataHost(const fs::path& path, uint32_t offset, uint32_t length,
- uint64_t address, bool upstream);
-} // namespace dma
+class DMA
+{
+ public:
+ /** @brief API to transfer data between BMC and host using DMA
+ *
+ * @param[in] path - pathname of the file to transfer data from or to
+ * @param[in] offset - offset in the file
+ * @param[in] length - length of the data to transfer
+ * @param[in] address - DMA address on the host
+ * @param[in] upstream - indicates direction of the transfer; true indicates
+ * transfer to the host
+ *
+ * @return returns 0 on success, negative errno on failure
+ */
+ int transferDataHost(const fs::path& path, uint32_t offset, uint32_t length,
+ uint64_t address, bool upstream);
+};
/** @brief Transfer the data between BMC and host using DMA.
*
@@ -83,6 +95,8 @@
* and the requested length is broken down into multiple DMA operations if the
* length exceed max size.
*
+ * @tparam[in] T - DMA interface type
+ * @param[in] intf - interface passed to invoke DMA transfer
* @param[in] command - PLDM command
* @param[in] path - pathname of the file to transfer data from or to
* @param[in] offset - offset in the file
@@ -92,9 +106,41 @@
* transfer to the host
* @param[out] response - response message location
*/
-void transferAll(uint8_t command, fs::path& path, uint32_t offset,
- uint32_t length, uint64_t address, bool upstream,
- pldm_msg* response);
+
+template <class DMAInterface>
+void transferAll(DMAInterface* intf, uint8_t command, fs::path& path,
+ uint32_t offset, uint32_t length, uint64_t address,
+ bool upstream, pldm_msg* response)
+{
+ uint32_t origLength = length;
+
+ while (length > dma::maxSize)
+ {
+ auto rc = intf->transferDataHost(path, offset, dma::maxSize, address,
+ upstream);
+ if (rc < 0)
+ {
+ encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, response);
+ return;
+ }
+
+ offset += dma::maxSize;
+ length -= dma::maxSize;
+ address += dma::maxSize;
+ }
+
+ auto rc = intf->transferDataHost(path, offset, length, address, upstream);
+ if (rc < 0)
+ {
+ encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, response);
+ return;
+ }
+
+ encode_rw_file_memory_resp(0, command, PLDM_SUCCESS, origLength, response);
+ return;
+}
+
+} // namespace dma
/** @brief Handler for readFileIntoMemory command
*
diff --git a/test/Makefile.am b/test/Makefile.am
index f854cec..70da63a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -3,12 +3,14 @@
TESTS = $(check_PROGRAMS)
check_PROGRAMS = \
- libpldm_fileio_test
+ libpldmoem_fileio_test \
+ libpldmoemresponder_fileio_test
test_cppflags = \
-Igtest \
$(GTEST_CPPFLAGS) \
- $(AM_CPPFLAGS)
+ $(AM_CPPFLAGS) \
+ $(PHOSPHOR_LOGGING_CFLAGS)
test_cxxflags = \
$(PTHREAD_CFLAGS)
@@ -17,11 +19,25 @@
-lgtest_main \
-lgtest \
$(PTHREAD_LIBS) \
- $(OESDK_TESTCASE_FLAGS)
+ $(OESDK_TESTCASE_FLAGS) \
+ $(PHOSPHOR_LOGGING_LIBS) \
+ -lstdc++fs \
+ -lgmock
-libpldm_fileio_test_CPPFLAGS = $(test_cppflags)
-libpldm_fileio_test_CXXFLAGS = $(test_cxxflags)
-libpldm_fileio_test_LDFLAGS = $(test_ldflags)
-libpldm_fileio_test_LDADD = $(top_builddir)/libpldm/base.o \
- $(top_builddir)/libpldm/file_io.o
-libpldm_fileio_test_SOURCES = libpldm_fileio_test.cpp
+libpldmoem_fileio_test_CPPFLAGS = $(test_cppflags)
+libpldmoem_fileio_test_CXXFLAGS = $(test_cxxflags)
+libpldmoem_fileio_test_LDFLAGS = $(test_ldflags)
+libpldmoem_fileio_test_LDADD = \
+ $(top_builddir)/libpldm/base.o \
+ $(top_builddir)/libpldm/file_io.o
+libpldmoem_fileio_test_SOURCES = libpldm_fileio_test.cpp
+
+libpldmoemresponder_fileio_test_CPPFLAGS = $(test_cppflags)
+libpldmoemresponder_fileio_test_CXXFLAGS = $(test_cxxflags)
+libpldmoemresponder_fileio_test_LDFLAGS = $(test_ldflags)
+libpldmoemresponder_fileio_test_LDADD = \
+ $(top_builddir)/libpldm/base.o \
+ $(top_builddir)/libpldm/file_io.o \
+ $(top_builddir)/libpldmresponder/file_io.o
+libpldmoemresponder_fileio_test_SOURCES = libpldmresponder_fileio_test.cpp
+
diff --git a/test/libpldmresponder_fileio_test.cpp b/test/libpldmresponder_fileio_test.cpp
new file mode 100644
index 0000000..93f5844
--- /dev/null
+++ b/test/libpldmresponder_fileio_test.cpp
@@ -0,0 +1,193 @@
+#include "libpldmresponder/file_io.hpp"
+
+#include "libpldm/base.h"
+#include "libpldm/file_io.h"
+
+#include <gmock/gmock-matchers.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#define SD_JOURNAL_SUPPRESS_LOCATION
+
+#include <systemd/sd-journal.h>
+
+std::vector<std::string> logs;
+
+extern "C" {
+
+int sd_journal_send(const char* format, ...)
+{
+ logs.push_back(format);
+ return 0;
+}
+
+int sd_journal_send_with_location(const char* file, const char* line,
+ const char* func, const char* format, ...)
+{
+ logs.push_back(format);
+ return 0;
+}
+}
+
+namespace pldm
+{
+
+namespace responder
+{
+
+namespace dma
+{
+
+class MockDMA
+{
+ public:
+ MOCK_METHOD5(transferDataHost,
+ int(const fs::path& file, uint32_t offset, uint32_t length,
+ uint64_t address, bool upstream));
+};
+
+} // namespace dma
+} // namespace responder
+} // namespace pldm
+using namespace pldm::responder;
+using ::testing::_;
+using ::testing::Return;
+
+TEST(TransferDataHost, GoodPath)
+{
+ using namespace pldm::responder::dma;
+
+ MockDMA dmaObj;
+ std::array<uint8_t, sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES>
+ responseMsg{};
+ pldm_msg* response = reinterpret_cast<pldm_msg*>(responseMsg.data());
+ fs::path path("");
+
+ // Minimum length of 16 and expect transferDataHost to be called once
+ // returns the default value of 0 (the return type of transferDataHost is
+ // int, the default value for int is 0)
+ uint32_t length = minSize;
+ EXPECT_CALL(dmaObj, transferDataHost(path, 0, length, 0, true)).Times(1);
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(response->payload + sizeof(response->payload[0]),
+ &length, sizeof(length)));
+
+ // maxsize of DMA
+ length = maxSize;
+ EXPECT_CALL(dmaObj, transferDataHost(path, 0, length, 0, true)).Times(1);
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(response->payload + sizeof(response->payload[0]),
+ &length, sizeof(length)));
+
+ // length greater than maxsize of DMA
+ length = maxSize + minSize;
+ EXPECT_CALL(dmaObj, transferDataHost(path, 0, maxSize, 0, true)).Times(1);
+ EXPECT_CALL(dmaObj, transferDataHost(path, maxSize, minSize, maxSize, true))
+ .Times(1);
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(response->payload + sizeof(response->payload[0]),
+ &length, sizeof(length)));
+
+ // length greater than 2*maxsize of DMA
+ length = 3 * maxSize;
+ EXPECT_CALL(dmaObj, transferDataHost(_, _, _, _, true)).Times(3);
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(response->payload + sizeof(response->payload[0]),
+ &length, sizeof(length)));
+
+ // check for downstream(copy data from host to BMC) parameter
+ length = minSize;
+ EXPECT_CALL(dmaObj, transferDataHost(path, 0, length, 0, false)).Times(1);
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, false, response);
+ ASSERT_EQ(response->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(response->payload + sizeof(response->payload[0]),
+ &length, sizeof(length)));
+}
+
+TEST(TransferDataHost, BadPath)
+{
+ using namespace pldm::responder::dma;
+
+ MockDMA dmaObj;
+ std::array<uint8_t, sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES>
+ responseMsg{};
+ pldm_msg* response = reinterpret_cast<pldm_msg*>(responseMsg.data());
+ fs::path path("");
+
+ // Minimum length of 16 and transferDataHost returning a negative errno
+ uint32_t length = minSize;
+ EXPECT_CALL(dmaObj, transferDataHost(_, _, _, _, _)).WillOnce(Return(-1));
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_ERROR);
+
+ // length greater than maxsize of DMA and transferDataHost returning a
+ // negative errno
+ length = maxSize + minSize;
+ EXPECT_CALL(dmaObj, transferDataHost(_, _, _, _, _)).WillOnce(Return(-1));
+ transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path, 0, length,
+ 0, true, response);
+ ASSERT_EQ(response->payload[0], PLDM_ERROR);
+}
+
+TEST(ReadFileIntoMemory, BadPath)
+{
+ uint32_t fileHandle = 0;
+ uint32_t offset = 0;
+ uint32_t length = 10;
+ uint64_t address = 0;
+
+ std::array<uint8_t, PLDM_RW_FILE_MEM_REQ_BYTES> requestMsg{};
+ memcpy(requestMsg.data(), &fileHandle, sizeof(fileHandle));
+ memcpy(requestMsg.data() + sizeof(fileHandle), &offset, sizeof(offset));
+ memcpy(requestMsg.data() + sizeof(fileHandle) + sizeof(offset), &length,
+ sizeof(length));
+ memcpy(requestMsg.data() + sizeof(fileHandle) + sizeof(offset) +
+ sizeof(length),
+ &address, sizeof(address));
+
+ std::array<uint8_t, sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES>
+ responseMsg{};
+ pldm_msg* response = reinterpret_cast<pldm_msg*>(responseMsg.data());
+
+ // Pass invalid payload length
+ readFileIntoMemory(requestMsg.data(), 0, response);
+ ASSERT_EQ(response->payload[0], PLDM_ERROR_INVALID_LENGTH);
+}
+
+TEST(WriteFileFromMemory, BadPath)
+{
+ uint32_t fileHandle = 0;
+ uint32_t offset = 0;
+ uint32_t length = 10;
+ uint64_t address = 0;
+
+ std::array<uint8_t, PLDM_RW_FILE_MEM_REQ_BYTES> requestMsg{};
+ memcpy(requestMsg.data(), &fileHandle, sizeof(fileHandle));
+ memcpy(requestMsg.data() + sizeof(fileHandle), &offset, sizeof(offset));
+ memcpy(requestMsg.data() + sizeof(fileHandle) + sizeof(offset), &length,
+ sizeof(length));
+ memcpy(requestMsg.data() + sizeof(fileHandle) + sizeof(offset) +
+ sizeof(length),
+ &address, sizeof(address));
+
+ std::array<uint8_t, sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES>
+ responseMsg{};
+ pldm_msg* response = reinterpret_cast<pldm_msg*>(responseMsg.data());
+
+ // Pass invalid payload length
+ writeFileFromMemory(requestMsg.data(), 0, response);
+ ASSERT_EQ(response->payload[0], PLDM_ERROR_INVALID_LENGTH);
+
+ // The length field is not a multiple of DMA minsize
+ writeFileFromMemory(requestMsg.data(), requestMsg.size(), response);
+ ASSERT_EQ(response->payload[0], PLDM_INVALID_WRITE_LENGTH);
+}