dsp: firmware_update: pack decomposed parameters to struct
There're two APIs that have decomposed parameters:
`encode_query_downstream_identifiers_req()` and
`encode_get_downstream_firmware_params_req(),
which against the checklist of API/ABI stabilization,
squashed those parameters to a struct to meet the request.
Change-Id: Ia952251cf8dcaeba060985e759e1d7aadf7b5b4d
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 548cc8d..0ef675c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,11 @@
Those downstream device related ABIs have not been stabilized yet, change
return type from PLDM Completion Code to ERRNO
+6. dsp: firmware_update: pack decomposed parameters to struct
+
+ `encode_query_downstream_identifiers_req()` and
+ `encode_get_downstream_firmware_params_req()`
+
### Fixed
1. dsp: platform: Fix location of closing paren in overflow detection
diff --git a/include/libpldm/firmware_update.h b/include/libpldm/firmware_update.h
index 3232006..f5d93fa 100644
--- a/include/libpldm/firmware_update.h
+++ b/include/libpldm/firmware_update.h
@@ -1173,9 +1173,9 @@
* 'msg.payload'
*/
int encode_query_downstream_identifiers_req(
- uint8_t instance_id, uint32_t data_transfer_handle,
- enum transfer_op_flag transfer_operation_flag, struct pldm_msg *msg,
- size_t payload_length);
+ uint8_t instance_id,
+ const struct pldm_query_downstream_identifiers_req *params_req,
+ struct pldm_msg *msg, size_t payload_length);
/**
* @brief Decodes the response message for Querying Downstream Identifiers.
@@ -1211,9 +1211,9 @@
* 'msg.payload'
*/
int encode_get_downstream_firmware_params_req(
- uint8_t instance_id, uint32_t data_transfer_handle,
- enum transfer_op_flag transfer_operation_flag, struct pldm_msg *msg,
- size_t payload_length);
+ uint8_t instance_id,
+ const struct pldm_get_downstream_firmware_params_req *params_req,
+ struct pldm_msg *msg, size_t payload_length);
/**
* @brief Decode response message for Get Downstream Firmware Parameters
diff --git a/src/dsp/firmware_update.c b/src/dsp/firmware_update.c
index d2b068a..59c5c20 100644
--- a/src/dsp/firmware_update.c
+++ b/src/dsp/firmware_update.c
@@ -980,19 +980,21 @@
LIBPLDM_ABI_TESTING
int encode_query_downstream_identifiers_req(
- uint8_t instance_id, uint32_t data_transfer_handle,
- enum transfer_op_flag transfer_operation_flag, struct pldm_msg *msg,
- size_t payload_length)
+ uint8_t instance_id,
+ const struct pldm_query_downstream_identifiers_req *params_req,
+ struct pldm_msg *msg, size_t payload_length)
{
struct pldm_msgbuf _buf;
struct pldm_msgbuf *buf = &_buf;
int rc;
- if (msg == NULL) {
+ if (!msg || !params_req) {
return -EINVAL;
}
- if (!is_transfer_operation_flag_valid(transfer_operation_flag)) {
+ if (!is_transfer_operation_flag_valid(
+ (enum transfer_op_flag)
+ params_req->transfer_operation_flag)) {
return -EINVAL;
}
@@ -1013,9 +1015,9 @@
return rc;
}
- pldm_msgbuf_insert(buf, data_transfer_handle);
+ pldm_msgbuf_insert(buf, params_req->data_transfer_handle);
// Data correctness has been verified, cast it to 1-byte data directly.
- pldm_msgbuf_insert(buf, (uint8_t)transfer_operation_flag);
+ pldm_msgbuf_insert(buf, params_req->transfer_operation_flag);
return pldm_msgbuf_destroy(buf);
}
@@ -1111,15 +1113,15 @@
LIBPLDM_ABI_TESTING
int encode_get_downstream_firmware_params_req(
- uint8_t instance_id, uint32_t data_transfer_handle,
- enum transfer_op_flag transfer_operation_flag, struct pldm_msg *msg,
- size_t payload_length)
+ uint8_t instance_id,
+ const struct pldm_get_downstream_firmware_params_req *params_req,
+ struct pldm_msg *msg, size_t payload_length)
{
struct pldm_msgbuf _buf;
struct pldm_msgbuf *buf = &_buf;
int rc;
- if (msg == NULL) {
+ if (!msg || !params_req) {
return -EINVAL;
}
@@ -1130,7 +1132,9 @@
return rc;
}
- if (!is_transfer_operation_flag_valid(transfer_operation_flag)) {
+ if (!is_transfer_operation_flag_valid(
+ (enum transfer_op_flag)
+ params_req->transfer_operation_flag)) {
return -EBADMSG;
}
@@ -1144,9 +1148,9 @@
return rc;
}
- pldm_msgbuf_insert(buf, data_transfer_handle);
+ pldm_msgbuf_insert(buf, params_req->data_transfer_handle);
// Data correctness has been verified, cast it to 1-byte data directly.
- pldm_msgbuf_insert(buf, (uint8_t)transfer_operation_flag);
+ pldm_msgbuf_insert(buf, params_req->transfer_operation_flag);
return pldm_msgbuf_destroy(buf);
}
diff --git a/tests/dsp/firmware_update.cpp b/tests/dsp/firmware_update.cpp
index 6fc1aa9..fe71343 100644
--- a/tests/dsp/firmware_update.cpp
+++ b/tests/dsp/firmware_update.cpp
@@ -1471,14 +1471,13 @@
TEST(QueryDownstreamIdentifiers, goodPathEncodeRequest)
{
constexpr uint8_t instanceId = 1;
- constexpr uint32_t dataTransferHandle = 0xFFFFFFFF;
- constexpr enum transfer_op_flag transferOperationFlag = PLDM_GET_FIRSTPART;
constexpr size_t payloadLen = PLDM_QUERY_DOWNSTREAM_IDENTIFIERS_REQ_BYTES;
PLDM_MSG_DEFINE_P(request, payloadLen);
+ constexpr pldm_query_downstream_identifiers_req params_req{
+ 0xFFFFFFFF, PLDM_GET_FIRSTPART};
- auto rc = encode_query_downstream_identifiers_req(
- instanceId, dataTransferHandle, transferOperationFlag, request,
- payloadLen);
+ auto rc = encode_query_downstream_identifiers_req(instanceId, ¶ms_req,
+ request, payloadLen);
ASSERT_EQ(rc, 0);
EXPECT_THAT(std::span<uint8_t>(request_buf, sizeof(request_buf)),
ElementsAreArray<uint8_t>(
@@ -1490,29 +1489,26 @@
TEST(QueryDownstreamIdentifiers, encodeRequestInvalidErrorPaths)
{
constexpr uint8_t instanceId = 1;
- constexpr uint32_t dataTransferHandle = 0x0;
- constexpr enum transfer_op_flag transferOperationFlag = PLDM_GET_FIRSTPART;
- constexpr enum transfer_op_flag invalidTransferOperationFlag =
- PLDM_ACKNOWLEDGEMENT_ONLY;
+ constexpr pldm_query_downstream_identifiers_req params_req{
+ 0xFFFFFFFF, PLDM_GET_FIRSTPART};
+ constexpr pldm_query_downstream_identifiers_req params_req_invalid{
+ 0xFFFFFFFF, PLDM_ACKNOWLEDGEMENT_ONLY};
constexpr size_t payload_length =
PLDM_QUERY_DOWNSTREAM_IDENTIFIERS_REQ_BYTES;
std::array<uint8_t, hdrSize + payload_length> requestMsg{};
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
auto requestPtr = reinterpret_cast<pldm_msg*>(requestMsg.data());
- auto rc = encode_query_downstream_identifiers_req(
- instanceId, dataTransferHandle, transferOperationFlag, nullptr,
- payload_length);
+ auto rc = encode_query_downstream_identifiers_req(instanceId, ¶ms_req,
+ nullptr, payload_length);
EXPECT_EQ(rc, -EINVAL);
rc = encode_query_downstream_identifiers_req(
- instanceId, dataTransferHandle, transferOperationFlag, requestPtr,
- payload_length - 1);
+ instanceId, ¶ms_req, requestPtr, payload_length - 1);
EXPECT_EQ(rc, -EOVERFLOW);
- rc = encode_query_downstream_identifiers_req(instanceId, dataTransferHandle,
- invalidTransferOperationFlag,
- requestPtr, payload_length);
+ rc = encode_query_downstream_identifiers_req(
+ instanceId, ¶ms_req_invalid, requestPtr, payload_length);
EXPECT_EQ(rc, -EINVAL);
}
#endif
@@ -2156,8 +2152,8 @@
TEST(GetDownstreamFirmwareParameters, goodPathEncodeRequest)
{
constexpr uint8_t instanceId = 1;
- constexpr uint32_t dataTransferHandle = 0x0;
- constexpr enum transfer_op_flag transferOperationFlag = PLDM_GET_FIRSTPART;
+ constexpr pldm_get_downstream_firmware_params_req params_req{
+ 0x0, PLDM_GET_FIRSTPART};
constexpr size_t payload_length =
PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMS_REQ_BYTES;
std::array<uint8_t, sizeof(pldm_msg_hdr) + payload_length> requestMsg{};
@@ -2165,8 +2161,7 @@
auto requestPtr = reinterpret_cast<pldm_msg*>(requestMsg.data());
auto rc = encode_get_downstream_firmware_params_req(
- instanceId, dataTransferHandle, transferOperationFlag, requestPtr,
- payload_length);
+ instanceId, ¶ms_req, requestPtr, payload_length);
EXPECT_EQ(rc, 0);
std::array<uint8_t, hdrSize + PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMS_REQ_BYTES>
@@ -2179,10 +2174,9 @@
TEST(GetDownstreamFirmwareParameters, encodeRequestInvalidTransferOperationFlag)
{
constexpr uint8_t instanceId = 1;
- constexpr uint32_t dataTransferHandle = 0x0;
// Setup invalid transfer operation flag
- constexpr enum transfer_op_flag transferOperationFlag =
- PLDM_ACKNOWLEDGEMENT_ONLY;
+ constexpr pldm_get_downstream_firmware_params_req params_req{
+ 0x0, PLDM_ACKNOWLEDGEMENT_ONLY};
constexpr size_t payload_length =
PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMS_REQ_BYTES;
std::array<uint8_t, sizeof(pldm_msg_hdr) + payload_length> requestMsg{};
@@ -2190,8 +2184,7 @@
auto requestPtr = reinterpret_cast<pldm_msg*>(requestMsg.data());
auto rc = encode_get_downstream_firmware_params_req(
- instanceId, dataTransferHandle, transferOperationFlag, requestPtr,
- payload_length);
+ instanceId, ¶ms_req, requestPtr, payload_length);
EXPECT_EQ(rc, -EBADMSG);
}
#endif
@@ -2200,10 +2193,9 @@
TEST(GetDownstreamFirmwareParameters, encodeRequestErrorBufSize)
{
constexpr uint8_t instanceId = 1;
- constexpr uint32_t dataTransferHandle = 0x0;
// Setup invalid transfer operation flag
- constexpr enum transfer_op_flag transferOperationFlag =
- PLDM_ACKNOWLEDGEMENT_ONLY;
+ constexpr pldm_get_downstream_firmware_params_req params_req{
+ 0x0, PLDM_ACKNOWLEDGEMENT_ONLY};
constexpr size_t payload_length =
PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMS_REQ_BYTES -
1 /* inject erro length*/;
@@ -2213,8 +2205,7 @@
auto requestPtr = reinterpret_cast<pldm_msg*>(requestMsg.data());
auto rc = encode_get_downstream_firmware_params_req(
- instanceId, dataTransferHandle, transferOperationFlag, requestPtr,
- payload_length);
+ instanceId, ¶ms_req, requestPtr, payload_length);
EXPECT_EQ(rc, -EOVERFLOW);
}
#endif