Refactor the responder functions for file I/O
The ReadFileIntoMemory and WriteFileFromMemory command's responder
function is refactored to return the PLDM response message. The caller
cannot know the space for the response buffer if the responder has a
variable length of response.
Change-Id: I81f649098a7711772b35ae3d0d8d23c7cd24e7e7
Signed-off-by: Tom Joseph <tomjoseph@in.ibm.com>
diff --git a/libpldmresponder/file_io.cpp b/libpldmresponder/file_io.cpp
index adcccac..9cf093c 100644
--- a/libpldmresponder/file_io.cpp
+++ b/libpldmresponder/file_io.cpp
@@ -126,8 +126,7 @@
} // namespace dma
-void readFileIntoMemory(const uint8_t* request, size_t payloadLength,
- pldm_msg* response)
+Response readFileIntoMemory(const uint8_t* request, size_t payloadLength)
{
uint32_t fileHandle = 0;
uint32_t offset = 0;
@@ -135,11 +134,14 @@
uint64_t address = 0;
fs::path path("");
+ Response response((sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES), 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+
if (payloadLength != PLDM_RW_FILE_MEM_REQ_BYTES)
{
encode_rw_file_memory_resp(0, PLDM_READ_FILE_INTO_MEMORY,
- PLDM_ERROR_INVALID_LENGTH, 0, response);
- return;
+ PLDM_ERROR_INVALID_LENGTH, 0, responsePtr);
+ return response;
}
decode_rw_file_memory_req(request, payloadLength, &fileHandle, &offset,
@@ -149,8 +151,8 @@
{
log<level::ERR>("File does not exist", entry("HANDLE=%d", fileHandle));
encode_rw_file_memory_resp(0, PLDM_READ_FILE_INTO_MEMORY,
- PLDM_INVALID_FILE_HANDLE, 0, response);
- return;
+ PLDM_INVALID_FILE_HANDLE, 0, responsePtr);
+ return response;
}
auto fileSize = fs::file_size(path);
@@ -159,8 +161,8 @@
log<level::ERR>("Offset exceeds file size", entry("OFFSET=%d", offset),
entry("FILE_SIZE=%d", fileSize));
encode_rw_file_memory_resp(0, PLDM_READ_FILE_INTO_MEMORY,
- PLDM_DATA_OUT_OF_RANGE, 0, response);
- return;
+ PLDM_DATA_OUT_OF_RANGE, 0, responsePtr);
+ return response;
}
if (offset + length > fileSize)
@@ -173,18 +175,17 @@
log<level::ERR>("Read length is not a multiple of DMA minSize",
entry("LENGTH=%d", length));
encode_rw_file_memory_resp(0, PLDM_READ_FILE_INTO_MEMORY,
- PLDM_INVALID_READ_LENGTH, 0, response);
- return;
+ PLDM_INVALID_READ_LENGTH, 0, responsePtr);
+ return response;
}
using namespace dma;
DMA intf;
- transferAll<DMA>(&intf, PLDM_READ_FILE_INTO_MEMORY, path, offset, length,
- address, true, response);
+ return transferAll<DMA>(&intf, PLDM_READ_FILE_INTO_MEMORY, path, offset,
+ length, address, true);
}
-void writeFileFromMemory(const uint8_t* request, size_t payloadLength,
- pldm_msg* response)
+Response writeFileFromMemory(const uint8_t* request, size_t payloadLength)
{
uint32_t fileHandle = 0;
uint32_t offset = 0;
@@ -192,11 +193,14 @@
uint64_t address = 0;
fs::path path("");
+ Response response(sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+
if (payloadLength != PLDM_RW_FILE_MEM_REQ_BYTES)
{
encode_rw_file_memory_resp(0, PLDM_WRITE_FILE_FROM_MEMORY,
- PLDM_ERROR_INVALID_LENGTH, 0, response);
- return;
+ PLDM_ERROR_INVALID_LENGTH, 0, responsePtr);
+ return response;
}
decode_rw_file_memory_req(request, payloadLength, &fileHandle, &offset,
@@ -207,16 +211,16 @@
log<level::ERR>("Write length is not a multiple of DMA minSize",
entry("LENGTH=%d", length));
encode_rw_file_memory_resp(0, PLDM_WRITE_FILE_FROM_MEMORY,
- PLDM_INVALID_WRITE_LENGTH, 0, response);
- return;
+ PLDM_INVALID_WRITE_LENGTH, 0, responsePtr);
+ return response;
}
if (!fs::exists(path))
{
log<level::ERR>("File does not exist", entry("HANDLE=%d", fileHandle));
encode_rw_file_memory_resp(0, PLDM_WRITE_FILE_FROM_MEMORY,
- PLDM_INVALID_FILE_HANDLE, 0, response);
- return;
+ PLDM_INVALID_FILE_HANDLE, 0, responsePtr);
+ return response;
}
auto fileSize = fs::file_size(path);
@@ -225,14 +229,14 @@
log<level::ERR>("Offset exceeds file size", entry("OFFSET=%d", offset),
entry("FILE_SIZE=%d", fileSize));
encode_rw_file_memory_resp(0, PLDM_WRITE_FILE_FROM_MEMORY,
- PLDM_DATA_OUT_OF_RANGE, 0, response);
- return;
+ PLDM_DATA_OUT_OF_RANGE, 0, responsePtr);
+ return response;
}
using namespace dma;
DMA intf;
- transferAll<DMA>(&intf, PLDM_WRITE_FILE_FROM_MEMORY, path, offset, length,
- address, false, response);
+ return transferAll<DMA>(&intf, PLDM_WRITE_FILE_FROM_MEMORY, path, offset,
+ length, address, false);
}
} // namespace responder
diff --git a/libpldmresponder/file_io.hpp b/libpldmresponder/file_io.hpp
index ea3a932..17a6fe2 100644
--- a/libpldmresponder/file_io.hpp
+++ b/libpldmresponder/file_io.hpp
@@ -14,6 +14,8 @@
namespace responder
{
+using Response = std::vector<uint8_t>;
+
namespace utils
{
@@ -104,15 +106,17 @@
* @param[in] address - DMA address on the host
* @param[in] upstream - indicates direction of the transfer; true indicates
* transfer to the host
- * @param[out] response - response message location
+ * @return PLDM response message
*/
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)
+Response transferAll(DMAInterface* intf, uint8_t command, fs::path& path,
+ uint32_t offset, uint32_t length, uint64_t address,
+ bool upstream)
{
uint32_t origLength = length;
+ Response response(sizeof(pldm_msg_hdr) + PLDM_RW_FILE_MEM_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
while (length > dma::maxSize)
{
@@ -120,8 +124,8 @@
upstream);
if (rc < 0)
{
- encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, response);
- return;
+ encode_rw_file_memory_resp(0, command, PLDM_ERROR, 0, responsePtr);
+ return response;
}
offset += dma::maxSize;
@@ -132,12 +136,13 @@
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_ERROR, 0, responsePtr);
+ return response;
}
- encode_rw_file_memory_resp(0, command, PLDM_SUCCESS, origLength, response);
- return;
+ encode_rw_file_memory_resp(0, command, PLDM_SUCCESS, origLength,
+ responsePtr);
+ return response;
}
} // namespace dma
@@ -146,19 +151,18 @@
*
* @param[in] request - pointer to PLDM request payload
* @param[in] payloadLength - length of the message payload
- * @param[out] response - response message location
+ *
+ * @return PLDM response message
*/
-void readFileIntoMemory(const uint8_t* request, size_t payloadLength,
- pldm_msg* response);
+Response readFileIntoMemory(const uint8_t* request, size_t payloadLength);
/** @brief Handler for writeFileIntoMemory command
*
* @param[in] request - pointer to PLDM request payload
* @param[in] payloadLength - length of the message payload
- * @param[out] response - response message location
+ *
+ * @return PLDM response message
*/
-void writeFileFromMemory(const uint8_t* request, size_t payloadLength,
- pldm_msg* response);
-
+Response writeFileFromMemory(const uint8_t* request, size_t payloadLength);
} // namespace responder
} // namespace pldm
diff --git a/test/libpldmresponder_fileio_test.cpp b/test/libpldmresponder_fileio_test.cpp
index 93f5844..060b0ed 100644
--- a/test/libpldmresponder_fileio_test.cpp
+++ b/test/libpldmresponder_fileio_test.cpp
@@ -57,9 +57,6 @@
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
@@ -67,19 +64,21 @@
// 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]),
+ auto response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY,
+ path, 0, length, 0, true);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(responsePtr->payload + sizeof(responsePtr->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]),
+ response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path,
+ 0, length, 0, true);
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(responsePtr->payload + sizeof(responsePtr->payload[0]),
&length, sizeof(length)));
// length greater than maxsize of DMA
@@ -87,28 +86,31 @@
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]),
+ response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path,
+ 0, length, 0, true);
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(responsePtr->payload + sizeof(responsePtr->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]),
+ response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path,
+ 0, length, 0, true);
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(responsePtr->payload + sizeof(responsePtr->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]),
+ response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path,
+ 0, length, 0, false);
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_SUCCESS);
+ ASSERT_EQ(0, memcmp(responsePtr->payload + sizeof(responsePtr->payload[0]),
&length, sizeof(length)));
}
@@ -117,25 +119,24 @@
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);
+ auto response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY,
+ path, 0, length, 0, true);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->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);
+ response = transferAll<MockDMA>(&dmaObj, PLDM_READ_FILE_INTO_MEMORY, path,
+ 0, length, 0, true);
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_ERROR);
}
TEST(ReadFileIntoMemory, BadPath)
@@ -154,13 +155,10 @@
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);
+ auto response = readFileIntoMemory(requestMsg.data(), 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_ERROR_INVALID_LENGTH);
}
TEST(WriteFileFromMemory, BadPath)
@@ -179,15 +177,13 @@
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);
+ auto response = writeFileFromMemory(requestMsg.data(), 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->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);
+ response = writeFileFromMemory(requestMsg.data(), requestMsg.size());
+ responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ ASSERT_EQ(responsePtr->payload[0], PLDM_INVALID_WRITE_LENGTH);
}