dsp: firmware_update: Add iterator for downstream device parameters
The previous attempt where we invented a struct that made it possible to
hold full-sized version strings was awkward on several fronts. Replace
it with an iterator in the style of the downstream device descriptors.
Change-Id: If9b83f4704b3068de9113af7451051c086f39969
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 64025cf..1a17350 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -58,6 +58,11 @@
marked as testing. There should be no impact on users of the stable APIs/
ABIs.
+8. Reimplement parsing of the firmware update downstream device parameter table
+ using an iterator macro
+
+ The change removes redundant APIs in the process.
+
### 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 0dfb3bc..c818347 100644
--- a/include/libpldm/firmware_update.h
+++ b/include/libpldm/firmware_update.h
@@ -795,28 +795,8 @@
char pending_comp_release_date[PLDM_FWUP_COMPONENT_RELEASE_DATA_LEN + 1];
bitfield16_t comp_activation_methods;
bitfield32_t capabilities_during_update;
- const char *active_comp_ver_str;
- const char *pending_comp_ver_str;
-};
-
-/** @struct pldm_downstream_device_parameter_entry_versions
- *
- * Structure representing downstream device parameter table entry with
- * copies of active and pending component version strings to avoid the
- * message buffer is subsequently freed.
- *
- * Clients should allocate memory for this struct then decode the response
- * instead of using `pldm_downstream_device_parameter_entry`.
- */
-struct pldm_downstream_device_parameter_entry_versions {
- struct pldm_downstream_device_parameters_entry entry;
- /* The "Length of ComponentVersionString" field is 1 byte, so
- * "ComponentVersionString" can be at most 255 characters, allocate
- * memory for it and append 1 bytes for null termination so that it
- * can be used as a Null-terminated string.
- */
- char active_comp_ver_str[UINT8_MAX + 1];
- char pending_comp_ver_str[UINT8_MAX + 1];
+ const void *active_comp_ver_str;
+ const void *pending_comp_ver_str;
};
/** @struct pldm_request_update_req
@@ -1215,6 +1195,11 @@
const struct pldm_get_downstream_firmware_parameters_req *params_req,
struct pldm_msg *msg, size_t payload_length);
+struct pldm_downstream_device_parameters_iter {
+ struct variable_field field;
+ size_t entries;
+};
+
/**
* @brief Decode response message for Get Downstream Firmware Parameters
*
@@ -1234,7 +1219,7 @@
int decode_get_downstream_firmware_parameters_resp(
const struct pldm_msg *msg, size_t payload_length,
struct pldm_get_downstream_firmware_parameters_resp *resp_data,
- struct variable_field *downstream_device_param_table);
+ struct pldm_downstream_device_parameters_iter *iter);
/**
* @brief Decode the next downstream device parameter table entry
@@ -1260,33 +1245,69 @@
* @note Caller is responsible for memory alloc and dealloc of param
* 'entry', 'active_comp_ver_str' and 'pending_comp_ver_str'
*/
-int decode_downstream_device_parameter_table_entry(
- struct variable_field *data,
- struct pldm_downstream_device_parameters_entry *entry,
- struct variable_field *versions);
+int decode_pldm_downstream_device_parameters_entry_from_iter(
+ struct pldm_downstream_device_parameters_iter *iter,
+ struct pldm_downstream_device_parameters_entry *entry);
-/**
- * @brief Decode the downstream device parameter table entry versions
+LIBPLDM_ITERATOR
+bool pldm_downstream_device_parameters_iter_end(
+ const struct pldm_downstream_device_parameters_iter *iter)
+{
+ return !iter->entries;
+}
+
+LIBPLDM_ITERATOR
+bool pldm_downstream_device_parameters_iter_next(
+ struct pldm_downstream_device_parameters_iter *iter)
+{
+ if (!iter->entries) {
+ return false;
+ }
+
+ iter->entries--;
+ return true;
+}
+
+/** @brief Iterator downstream device parameter entries in Get Downstream
+ * Firmware Parameters response
*
- * @param[in] versions - pointer to version strings raw data
- * @param[in,out] entry - pointer to the decoded downstream device parameter table entry
- * @param[out] active - pointer to active component version string container
- * @param[in] active_len - The size of the object pointed to by @p active
- * @param[out] pending - pointer to pending component version string container
- * @param[in] pending_len - The size of the object pointed to by @p pending
- * @return 0 on success, otherwise -EINVAL if the input parameters' memory
- * are not allocated, -EOVERFLOW if the payload length is not enough
- * to decode the message, -EBADMSG if the message is not valid.
+ * @param params The @ref "struct pldm_downstream_device_parameters_iter" lvalue
+ * used as the out-value from the corresponding call to @ref
+ * decode_get_downstream_firmware_parameters_resp
+ * @param entry The @ref "struct pldm_downstream_device_parameters_entry" lvalue
+ * into which the next parameter table entry should be decoded
+ * @param rc An lvalue of type int into which the return code from the decoding
+ * will be placed
*
- * @note Caller is responsible for memory alloc and dealloc of all the params,
- * and the param `entry` should be the instance which has successfully decoded
- * by `decode_downstream_device_parameter_table_entry()`.
+ * Example use of the macro is as follows:
*
+ * @code
+ * struct pldm_get_downstream_firmware_parameters_resp resp;
+ * struct pldm_downstream_device_parameters_iter params;
+ * struct pldm_downstream_device_parameters_entry entry;
+ * int rc;
+ *
+ * rc = decode_get_downstream_firmware_parameters_resp(..., &resp, ¶ms);
+ * if (rc) {
+ * // Handle any error from decoding the fixed-portion of response
+ * }
+ *
+ * foreach_pldm_downstream_device_parameters_entry(params, entry, rc) {
+ * // Do something with the decoded entry
+ * }
+ *
+ * if (rc) {
+ * // Handle any decoding error while iterating the variable-length set of
+ * //parameter entries
+ * }
+ * @endcode
*/
-int decode_downstream_device_parameter_table_entry_versions(
- const struct variable_field *versions,
- struct pldm_downstream_device_parameters_entry *entry, char *active,
- size_t active_len, char *pending, size_t pending_len);
+#define foreach_pldm_downstream_device_parameters_entry(params, entry, rc) \
+ for ((rc) = 0; \
+ (!pldm_downstream_device_parameters_iter_end(&(params)) && \
+ !((rc) = decode_pldm_downstream_device_parameters_entry_from_iter( \
+ &(params), &(entry)))); \
+ pldm_downstream_device_parameters_iter_next(&(params)))
/** @brief Create PLDM request message for RequestUpdate
*
diff --git a/src/dsp/firmware_update.c b/src/dsp/firmware_update.c
index c21c118..9254e48 100644
--- a/src/dsp/firmware_update.c
+++ b/src/dsp/firmware_update.c
@@ -1159,14 +1159,15 @@
int decode_get_downstream_firmware_parameters_resp(
const struct pldm_msg *msg, size_t payload_length,
struct pldm_get_downstream_firmware_parameters_resp *resp_data,
- struct variable_field *downstream_device_param_table)
+ struct pldm_downstream_device_parameters_iter *iter)
{
struct pldm_msgbuf _buf;
struct pldm_msgbuf *buf = &_buf;
+ void *remaining = NULL;
+ size_t length;
int rc;
- if (msg == NULL || resp_data == NULL ||
- downstream_device_param_table == NULL) {
+ if (msg == NULL || resp_data == NULL || iter == NULL) {
return -EINVAL;
}
@@ -1195,30 +1196,42 @@
resp_data->fdp_capabilities_during_update.value);
pldm_msgbuf_extract(buf, resp_data->downstream_device_count);
- return pldm_msgbuf_span_remaining(
- buf, (void **)&downstream_device_param_table->ptr,
- &downstream_device_param_table->length);
+ rc = pldm_msgbuf_span_remaining(buf, &remaining, &length);
+ if (rc) {
+ return rc;
+ }
+
+ rc = pldm_msgbuf_destroy(buf);
+ if (rc) {
+ return rc;
+ }
+
+ iter->field.ptr = remaining;
+ iter->field.length = length;
+ iter->entries = resp_data->downstream_device_count;
+
+ return 0;
}
LIBPLDM_ABI_TESTING
-int decode_downstream_device_parameter_table_entry(
- struct variable_field *data,
- struct pldm_downstream_device_parameters_entry *entry,
- struct variable_field *versions)
+int decode_pldm_downstream_device_parameters_entry_from_iter(
+ struct pldm_downstream_device_parameters_iter *iter,
+ struct pldm_downstream_device_parameters_entry *entry)
{
struct pldm_msgbuf _buf;
struct pldm_msgbuf *buf = &_buf;
- void *cursor = NULL;
+ void *comp_ver_str;
size_t remaining;
+ void *cursor;
int rc;
- if (data == NULL || entry == NULL || versions == NULL) {
+ if (iter == NULL || entry == NULL) {
return -EINVAL;
}
rc = pldm_msgbuf_init_errno(
- buf, PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN, data->ptr,
- data->length);
+ buf, PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN,
+ iter->field.ptr, iter->field.length);
if (rc < 0) {
return rc;
}
@@ -1263,81 +1276,29 @@
pldm_msgbuf_extract(buf, entry->comp_activation_methods.value);
pldm_msgbuf_extract(buf, entry->capabilities_during_update.value);
- const size_t versions_len = entry->active_comp_ver_str_len +
- entry->pending_comp_ver_str_len;
- rc = pldm_msgbuf_span_required(buf, versions_len,
- (void **)&versions->ptr);
- if (rc < 0) {
- return rc;
- }
- versions->length = versions_len;
+ comp_ver_str = NULL;
+ pldm_msgbuf_span_required(buf, entry->active_comp_ver_str_len,
+ &comp_ver_str);
+ entry->active_comp_ver_str = comp_ver_str;
+
+ comp_ver_str = NULL;
+ pldm_msgbuf_span_required(buf, entry->pending_comp_ver_str_len,
+ &comp_ver_str);
+ entry->pending_comp_ver_str = comp_ver_str;
+
+ cursor = NULL;
rc = pldm_msgbuf_span_remaining(buf, &cursor, &remaining);
if (rc < 0) {
return rc;
}
- data->ptr = cursor;
- data->length = remaining;
+ iter->field.ptr = cursor;
+ iter->field.length = remaining;
return 0;
}
-LIBPLDM_ABI_TESTING
-int decode_downstream_device_parameter_table_entry_versions(
- const struct variable_field *versions,
- struct pldm_downstream_device_parameters_entry *entry, char *active,
- size_t active_len, char *pending, size_t pending_len)
-{
- struct pldm_msgbuf _buf;
- struct pldm_msgbuf *buf = &_buf;
- int rc;
-
- if (versions == NULL || versions->ptr == NULL || !versions->length ||
- entry == NULL || active == NULL || pending == NULL) {
- return -EINVAL;
- }
-
- if (!active_len || active_len - 1 < entry->active_comp_ver_str_len) {
- return -EOVERFLOW;
- }
-
- if (!pending_len || pending_len - 1 < entry->pending_comp_ver_str_len) {
- return -EOVERFLOW;
- }
-
- /* This API should be called after decode_downstream_device_parameter_table_entry
- * has successfully decoded the entry, assume the entry data is valid here.
- */
- const size_t versions_len = entry->active_comp_ver_str_len +
- entry->pending_comp_ver_str_len;
- rc = pldm_msgbuf_init_errno(buf, versions_len, versions->ptr,
- versions->length);
- if (rc < 0) {
- return rc;
- }
-
- rc = pldm_msgbuf_extract_array(buf, entry->active_comp_ver_str_len,
- active, active_len);
- if (rc < 0) {
- return rc;
- }
-
- active[entry->active_comp_ver_str_len] = '\0';
- rc = pldm_msgbuf_extract_array(buf, entry->pending_comp_ver_str_len,
- pending, pending_len);
- if (rc < 0) {
- return rc;
- }
-
- pending[entry->pending_comp_ver_str_len] = '\0';
-
- entry->active_comp_ver_str = active;
- entry->pending_comp_ver_str = pending;
-
- return pldm_msgbuf_destroy_consumed(buf);
-}
-
LIBPLDM_ABI_STABLE
int encode_request_update_req(uint8_t instance_id, uint32_t max_transfer_size,
uint16_t num_of_comp,
diff --git a/tests/dsp/firmware_update.cpp b/tests/dsp/firmware_update.cpp
index ffb25ff..bfaa020 100644
--- a/tests/dsp/firmware_update.cpp
+++ b/tests/dsp/firmware_update.cpp
@@ -7,6 +7,7 @@
#include <algorithm>
#include <array>
#include <bitset>
+#include <cstddef>
#include <cstdint>
#include <cstring>
#include <span>
@@ -1592,7 +1593,7 @@
foreach_pldm_downstream_device(devs, dev, rc)
{
- ASSERT_TRUE(false);
+ FAIL();
}
ASSERT_NE(rc, 0);
}
@@ -2212,79 +2213,274 @@
#endif
#ifdef LIBPLDM_API_TESTING
-TEST(GetDownstreamFirmwareParameters, goodPathDecodeResponse)
+TEST(GetDownstreamFirmwareParameters, goodPathDecodeResponseOneEntry)
{
- /** Count is not fixed here taking it as 1, and the downstream device's
- * version strings length are set to 8
- */
constexpr uint16_t downstreamDeviceCount = 1;
constexpr uint8_t activeComponentVersionStringLength = 8;
constexpr uint8_t pendingComponentVersionStringLength = 8;
constexpr size_t downstreamDeviceParamTableLen =
- sizeof(pldm_component_parameter_entry) +
+ PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN +
activeComponentVersionStringLength +
pendingComponentVersionStringLength;
- constexpr uint8_t complition_code_resp = PLDM_SUCCESS;
+ constexpr uint8_t completion_code_resp = PLDM_SUCCESS;
constexpr uint32_t next_data_transfer_handle_resp = 0x0;
constexpr uint8_t transfer_flag_resp = PLDM_START_AND_END;
constexpr bitfield32_t fdp_capabilities_during_update = {.value = 0x0002};
+ constexpr size_t payload_len =
+ PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN +
+ downstreamDeviceParamTableLen;
- std::array<uint8_t,
- hdrSize + PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN +
- downstreamDeviceParamTableLen>
- responseMsg{};
-
- int rc = 0;
-
+ PLDM_MSG_DEFINE_P(response, payload_len);
struct pldm_msgbuf _buf;
struct pldm_msgbuf* buf = &_buf;
- rc = pldm_msgbuf_init_errno(buf, 0, responseMsg.data() + hdrSize,
- responseMsg.size() - hdrSize);
+ int rc = 0;
+
+ rc = pldm_msgbuf_init_errno(buf, 0, response->payload, payload_len);
EXPECT_EQ(rc, 0);
- pldm_msgbuf_insert_uint8(buf, complition_code_resp);
+ // Table 24
+ pldm_msgbuf_insert_uint8(buf, completion_code_resp);
pldm_msgbuf_insert_uint32(buf, next_data_transfer_handle_resp);
pldm_msgbuf_insert_uint8(buf, transfer_flag_resp);
+
+ // Table 25
pldm_msgbuf_insert_uint32(buf, fdp_capabilities_during_update.value);
pldm_msgbuf_insert_uint16(buf, downstreamDeviceCount);
- /** Filling paramter table, the correctness of the downstream devices data
- * is not checked in this test case so filling with 0xff
- */
- std::fill_n(responseMsg.data() + hdrSize +
- PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN,
- downstreamDeviceParamTableLen, 0xff);
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- auto table = reinterpret_cast<pldm_component_parameter_entry*>(
- responseMsg.data() + hdrSize +
- PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN);
- table->active_comp_ver_str_len = activeComponentVersionStringLength;
- table->pending_comp_ver_str_len = pendingComponentVersionStringLength;
+ // Table 26
+ pldm_msgbuf_insert_uint16(buf, 0);
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- auto response = reinterpret_cast<pldm_msg*>(responseMsg.data());
+ // - Active metadata
+ pldm_msgbuf_insert_uint32(buf, 0);
+ pldm_msgbuf_insert_uint8(buf, 1);
+ pldm_msgbuf_insert_uint8(buf, activeComponentVersionStringLength);
+ rc = pldm__msgbuf_insert_array_void(buf, 8, "20241206", 8);
+ ASSERT_EQ(rc, 0);
+
+ // - Pending metadata
+ pldm_msgbuf_insert_uint32(buf, 0);
+ pldm_msgbuf_insert_uint8(buf, 1);
+ pldm_msgbuf_insert_uint8(buf, pendingComponentVersionStringLength);
+ rc = pldm__msgbuf_insert_array_void(buf, 8, "20241206", 8);
+ ASSERT_EQ(rc, 0);
+
+ // - Methods and capabilities
+ pldm_msgbuf_insert_uint16(buf, 1);
+ pldm_msgbuf_insert_uint32(buf, 0);
+
+ // - Version strings
+ rc = pldm__msgbuf_insert_array_void(buf, activeComponentVersionStringLength,
+ "abcdefgh", 8);
+ ASSERT_EQ(rc, 0);
+ rc = pldm__msgbuf_insert_array_void(
+ buf, pendingComponentVersionStringLength, "zyxwvuts", 8);
+ ASSERT_EQ(rc, 0);
+
+ rc = pldm_msgbuf_destroy_consumed(buf);
+ ASSERT_EQ(rc, 0);
+
struct pldm_get_downstream_firmware_parameters_resp resp_data = {};
- struct variable_field downstreamDeviceParamTable = {};
+ struct pldm_downstream_device_parameters_iter iter = {};
- rc = decode_get_downstream_firmware_parameters_resp(
- response, responseMsg.size() - hdrSize, &resp_data,
- &downstreamDeviceParamTable);
+ rc = decode_get_downstream_firmware_parameters_resp(response, payload_len,
+ &resp_data, &iter);
- EXPECT_EQ(rc, 0);
- EXPECT_EQ(resp_data.completion_code, complition_code_resp);
+ ASSERT_EQ(rc, 0);
+ EXPECT_EQ(resp_data.completion_code, completion_code_resp);
EXPECT_EQ(resp_data.next_data_transfer_handle,
next_data_transfer_handle_resp);
EXPECT_EQ(resp_data.transfer_flag, transfer_flag_resp);
EXPECT_EQ(resp_data.downstream_device_count, downstreamDeviceCount);
- EXPECT_EQ(downstreamDeviceParamTable.length, downstreamDeviceParamTableLen);
- EXPECT_EQ(
- true,
- std::equal(downstreamDeviceParamTable.ptr,
- downstreamDeviceParamTable.ptr +
- downstreamDeviceParamTable.length,
- responseMsg.begin() + hdrSize +
- PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN,
- responseMsg.end()));
+
+ struct pldm_downstream_device_parameters_entry entry;
+ size_t entries = 0;
+ foreach_pldm_downstream_device_parameters_entry(iter, entry, rc)
+ {
+ EXPECT_EQ(entry.downstream_device_index, 0);
+ EXPECT_EQ(entry.active_comp_comparison_stamp, 0);
+ EXPECT_EQ(entry.active_comp_ver_str_type, 1);
+ EXPECT_EQ(entry.active_comp_ver_str_len,
+ activeComponentVersionStringLength);
+ EXPECT_STREQ("20241206", entry.active_comp_release_date);
+ EXPECT_EQ(entry.pending_comp_comparison_stamp, 0);
+ EXPECT_EQ(entry.pending_comp_ver_str_type, 1);
+ EXPECT_EQ(entry.pending_comp_ver_str_len,
+ pendingComponentVersionStringLength);
+ EXPECT_STREQ("20241206", entry.pending_comp_release_date);
+ EXPECT_EQ(entry.comp_activation_methods.value, 1);
+ EXPECT_EQ(entry.capabilities_during_update.value, 0);
+ EXPECT_FALSE(memcmp("abcdefgh", entry.active_comp_ver_str,
+ entry.active_comp_ver_str_len));
+ EXPECT_FALSE(memcmp("zyxwvuts", entry.pending_comp_ver_str,
+ entry.pending_comp_ver_str_len));
+ entries++;
+ }
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(entries, 1);
+}
+#endif
+
+#ifdef LIBPLDM_API_TESTING
+TEST(GetDownstreamFirmwareParameters, goodPathDecodeResponseTwoEntries)
+{
+ /** Count is not fixed here taking it as 1, and the downstream device's
+ * version strings length are set to 8
+ */
+ constexpr uint16_t downstreamDeviceCount = 2;
+ constexpr uint8_t activeComponentVersionStringLength = 8;
+ constexpr uint8_t pendingComponentVersionStringLength = 9;
+ constexpr size_t downstreamDeviceParamTableLen =
+ static_cast<size_t>(downstreamDeviceCount *
+ (PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN +
+ activeComponentVersionStringLength +
+ pendingComponentVersionStringLength));
+ constexpr uint8_t completion_code_resp = PLDM_SUCCESS;
+ constexpr uint32_t next_data_transfer_handle_resp = 0x0;
+ constexpr uint8_t transfer_flag_resp = PLDM_START_AND_END;
+ constexpr bitfield32_t fdp_capabilities_during_update = {.value = 0x0002};
+ constexpr size_t payload_len =
+ PLDM_GET_DOWNSTREAM_FIRMWARE_PARAMETERS_RESP_MIN_LEN +
+ downstreamDeviceParamTableLen;
+
+ PLDM_MSG_DEFINE_P(response, payload_len);
+ struct pldm_msgbuf _buf;
+ struct pldm_msgbuf* buf = &_buf;
+ int rc = 0;
+
+ rc = pldm_msgbuf_init_errno(buf, 0, response->payload, payload_len);
+ EXPECT_EQ(rc, 0);
+
+ // Table 24
+ pldm_msgbuf_insert_uint8(buf, completion_code_resp);
+ pldm_msgbuf_insert_uint32(buf, next_data_transfer_handle_resp);
+ pldm_msgbuf_insert_uint8(buf, transfer_flag_resp);
+
+ // Table 25
+ pldm_msgbuf_insert_uint32(buf, fdp_capabilities_during_update.value);
+ pldm_msgbuf_insert_uint16(buf, downstreamDeviceCount);
+
+ constexpr const std::array<pldm_downstream_device_parameters_entry, 2>
+ table = {{{
+ 0,
+ 0,
+ 1,
+ 8,
+ "20241206",
+ 0,
+ 1,
+ 9,
+ "20241209",
+ {1},
+ {0},
+ "active_0",
+ "pending_0",
+ },
+ {
+ 1,
+ 0,
+ 1,
+ 8,
+ "20241209",
+ 0,
+ 1,
+ 9,
+ "20241206",
+ {1},
+ {0},
+ "active_1",
+ "pending_1",
+ }}};
+ for (const auto& e : table)
+ {
+ // Table 26
+ pldm_msgbuf_insert_uint16(buf, e.downstream_device_index);
+
+ // - Active metadata
+ pldm_msgbuf_insert_uint32(buf, e.active_comp_comparison_stamp);
+ pldm_msgbuf_insert_uint8(buf, e.active_comp_ver_str_type);
+ pldm_msgbuf_insert_uint8(buf, e.active_comp_ver_str_len);
+ rc = pldm__msgbuf_insert_array_void(buf, 8, &e.active_comp_release_date,
+ sizeof(e.active_comp_release_date));
+ ASSERT_EQ(rc, 0);
+
+ // - Pending metadata
+ pldm_msgbuf_insert_uint32(buf, e.pending_comp_comparison_stamp);
+ pldm_msgbuf_insert_uint8(buf, e.pending_comp_ver_str_type);
+ pldm_msgbuf_insert_uint8(buf, e.pending_comp_ver_str_len);
+ rc =
+ pldm__msgbuf_insert_array_void(buf, 8, e.pending_comp_release_date,
+ sizeof(e.pending_comp_release_date));
+ ASSERT_EQ(rc, 0);
+
+ // - Methods and capabilities
+ pldm_msgbuf_insert_uint16(buf, e.comp_activation_methods.value);
+ pldm_msgbuf_insert_uint32(buf, e.capabilities_during_update.value);
+
+ // - Version strings
+ rc = pldm__msgbuf_insert_array_void(buf, e.active_comp_ver_str_len,
+ e.active_comp_ver_str,
+ e.active_comp_ver_str_len);
+ ASSERT_EQ(rc, 0);
+ rc = pldm__msgbuf_insert_array_void(buf, e.pending_comp_ver_str_len,
+ e.pending_comp_ver_str,
+ e.pending_comp_ver_str_len);
+ ASSERT_EQ(rc, 0);
+ }
+
+ rc = pldm_msgbuf_destroy_consumed(buf);
+ ASSERT_EQ(rc, 0);
+
+ struct pldm_get_downstream_firmware_parameters_resp resp_data = {};
+ struct pldm_downstream_device_parameters_iter iter = {};
+
+ rc = decode_get_downstream_firmware_parameters_resp(response, payload_len,
+ &resp_data, &iter);
+
+ ASSERT_EQ(rc, 0);
+ EXPECT_EQ(resp_data.completion_code, completion_code_resp);
+ EXPECT_EQ(resp_data.next_data_transfer_handle,
+ next_data_transfer_handle_resp);
+ EXPECT_EQ(resp_data.transfer_flag, transfer_flag_resp);
+ EXPECT_EQ(resp_data.downstream_device_count, downstreamDeviceCount);
+
+ struct pldm_downstream_device_parameters_entry entry;
+ size_t entryIndex = 0;
+ foreach_pldm_downstream_device_parameters_entry(iter, entry, rc)
+ {
+ ASSERT_LE(entryIndex, table.size());
+
+ EXPECT_EQ(table[entryIndex].downstream_device_index,
+ entry.downstream_device_index);
+ EXPECT_EQ(table[entryIndex].active_comp_comparison_stamp,
+ entry.active_comp_comparison_stamp);
+ EXPECT_EQ(table[entryIndex].active_comp_ver_str_type,
+ entry.active_comp_ver_str_type);
+ EXPECT_EQ(table[entryIndex].active_comp_ver_str_len,
+ entry.active_comp_ver_str_len);
+ EXPECT_STREQ(&table[entryIndex].active_comp_release_date[0],
+ &entry.active_comp_release_date[0]);
+ EXPECT_EQ(table[entryIndex].pending_comp_comparison_stamp,
+ entry.pending_comp_comparison_stamp);
+ EXPECT_EQ(table[entryIndex].pending_comp_ver_str_type,
+ entry.pending_comp_ver_str_type);
+ EXPECT_EQ(table[entryIndex].pending_comp_ver_str_len,
+ entry.pending_comp_ver_str_len);
+ EXPECT_STREQ(&table[entryIndex].pending_comp_release_date[0],
+ &entry.pending_comp_release_date[0]);
+ EXPECT_EQ(table[entryIndex].comp_activation_methods.value,
+ entry.comp_activation_methods.value);
+ EXPECT_EQ(table[entryIndex].capabilities_during_update.value,
+ entry.capabilities_during_update.value);
+ EXPECT_FALSE(memcmp(table[entryIndex].active_comp_ver_str,
+ entry.active_comp_ver_str,
+ table[entryIndex].active_comp_ver_str_len));
+ EXPECT_FALSE(memcmp(table[entryIndex].pending_comp_ver_str,
+ entry.pending_comp_ver_str,
+ table[entryIndex].pending_comp_ver_str_len));
+ entryIndex++;
+ }
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(entryIndex, table.size());
}
#endif
@@ -2336,236 +2532,18 @@
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
auto response = reinterpret_cast<pldm_msg*>(responseMsg.data());
struct pldm_get_downstream_firmware_parameters_resp resp_data = {};
- struct variable_field downstreamDeviceParamTable = {};
+ struct pldm_downstream_device_parameters_iter iter;
rc = decode_get_downstream_firmware_parameters_resp(
- response, responseMsg.size() - hdrSize, &resp_data,
- &downstreamDeviceParamTable);
+ response, responseMsg.size() - hdrSize, &resp_data, &iter);
EXPECT_EQ(rc, 0);
- pldm_downstream_device_parameters_entry entry{};
- variable_field versions{};
-
- EXPECT_NE(decode_downstream_device_parameter_table_entry(
- &downstreamDeviceParamTable, &entry, &versions),
- 0);
-}
-#endif
-
-#ifdef LIBPLDM_API_TESTING
-TEST(GetDownstreamFirmwareParameters, goodPathDecodeDownstreamDeviceParamTable)
-{
- // Arbitrary downstream device index
- constexpr uint16_t downstreamDeviceIndex = 1;
- // Arbitrary value for component classification
- constexpr uint32_t comparisonStamp = 0x12345678;
- // Arbitrary value for component activation methods
- constexpr uint16_t compActivationMethods = 0xbbdd;
- // Arbitrary value for capabilities during update
- constexpr uint32_t capabilitiesDuringUpdate = 0xbadbeefe;
- // ActiveCompImageSetVerStrLen is not fixed here taking it as 8
- constexpr uint8_t activeCompVerStrLen = 8;
- // PendingCompImageSetVerStrLen is not fixed here taking it as 8
- constexpr uint8_t pendingCompVerStrLen = 8;
- // Arbitrary value for release date
- constexpr char release_date[8] = {'2', '0', '2', '4', '0', '6', '2', '1'};
- // Arbitrary version strings
- constexpr char activeCompVerStr[activeCompVerStrLen] = {'1', '2', '3', '4',
- '5', '6', '7', '8'};
- constexpr char pendingCompVerStr[pendingCompVerStrLen] = {
- 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'};
-
- std::array<uint8_t, PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN +
- activeCompVerStrLen + pendingCompVerStrLen>
- responseMsg{};
-
- int rc = 0;
-
- struct pldm_msgbuf _buf;
- struct pldm_msgbuf* buf = &_buf;
- rc = pldm_msgbuf_init_errno(buf,
- PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN,
- responseMsg.data(), responseMsg.size());
- EXPECT_EQ(rc, 0);
-
- pldm_msgbuf_insert_uint16(buf, downstreamDeviceIndex);
- pldm_msgbuf_insert_uint32(buf, comparisonStamp);
- pldm_msgbuf_insert_uint8(buf, (uint8_t)PLDM_STR_TYPE_ASCII);
- pldm_msgbuf_insert_uint8(buf, activeCompVerStrLen);
- rc = pldm_msgbuf_insert_array_char(buf, sizeof(release_date), release_date,
- sizeof(release_date));
- ASSERT_EQ(rc, 0);
- pldm_msgbuf_insert_uint32(buf, comparisonStamp);
- pldm_msgbuf_insert_uint8(buf, (uint8_t)PLDM_STR_TYPE_ASCII);
- pldm_msgbuf_insert_uint8(buf, pendingCompVerStrLen);
- rc = pldm_msgbuf_insert_array_char(buf, sizeof(release_date), release_date,
- sizeof(release_date));
- ASSERT_EQ(rc, 0);
- pldm_msgbuf_insert_uint16(buf, compActivationMethods);
- pldm_msgbuf_insert_uint32(buf, capabilitiesDuringUpdate);
- rc = pldm_msgbuf_insert_array_char(
- buf, activeCompVerStrLen, activeCompVerStr, sizeof(activeCompVerStr));
- ASSERT_EQ(rc, 0);
- rc = pldm_msgbuf_insert_array_char(buf, pendingCompVerStrLen,
- pendingCompVerStr,
- sizeof(pendingCompVerStr));
- ASSERT_EQ(rc, 0);
-
- variable_field rawData = {.ptr = responseMsg.data(),
- .length = responseMsg.size()};
- struct pldm_downstream_device_parameter_entry_versions entry_version = {};
- struct variable_field versions = {};
- const uint8_t* original_ptr = rawData.ptr;
-
- rc = decode_downstream_device_parameter_table_entry(
- &rawData, &entry_version.entry, &versions);
-
- EXPECT_EQ(rc, 0);
- EXPECT_EQ(rawData.ptr, original_ptr +
- PLDM_DOWNSTREAM_DEVICE_PARAMETERS_ENTRY_MIN_LEN +
- entry_version.entry.active_comp_ver_str_len +
- entry_version.entry.pending_comp_ver_str_len);
- EXPECT_EQ(rawData.length, 0);
-
- // Further decode the version strings
- rc = decode_downstream_device_parameter_table_entry_versions(
- &versions, &entry_version.entry, entry_version.active_comp_ver_str,
- sizeof(entry_version.active_comp_ver_str),
- entry_version.pending_comp_ver_str,
- sizeof(entry_version.pending_comp_ver_str));
- struct pldm_downstream_device_parameters_entry entry = entry_version.entry;
- EXPECT_EQ(rc, 0);
-
- // Verify the decoded table entry
- EXPECT_EQ(entry.downstream_device_index, downstreamDeviceIndex);
- EXPECT_EQ(entry.active_comp_comparison_stamp, comparisonStamp);
- EXPECT_EQ(entry.active_comp_ver_str_type, PLDM_STR_TYPE_ASCII);
- EXPECT_EQ(entry.active_comp_ver_str_len, activeCompVerStrLen);
- EXPECT_EQ(0, memcmp(entry.active_comp_release_date, release_date,
- sizeof(release_date)));
- EXPECT_EQ(entry.pending_comp_comparison_stamp, comparisonStamp);
- EXPECT_EQ(entry.pending_comp_ver_str_type, PLDM_STR_TYPE_ASCII);
- EXPECT_EQ(entry.pending_comp_ver_str_len, pendingCompVerStrLen);
- EXPECT_EQ(0, memcmp(entry.pending_comp_release_date, release_date,
- sizeof(release_date)));
- EXPECT_EQ(entry.comp_activation_methods.value, compActivationMethods);
- EXPECT_EQ(entry.capabilities_during_update.value, capabilitiesDuringUpdate);
- EXPECT_EQ(entry.active_comp_ver_str_len + entry.pending_comp_ver_str_len,
- versions.length);
- EXPECT_EQ(0, memcmp(versions.ptr, activeCompVerStr, activeCompVerStrLen));
- EXPECT_EQ(0, memcmp(versions.ptr + entry.active_comp_ver_str_len,
- pendingCompVerStr, pendingCompVerStrLen));
-
- // Verify version strings
- EXPECT_EQ(0, memcmp(entry_version.entry.active_comp_ver_str,
- activeCompVerStr, activeCompVerStrLen));
- EXPECT_EQ('\0',
- entry_version.entry.active_comp_ver_str[activeCompVerStrLen]);
- EXPECT_EQ(0, memcmp(entry_version.entry.pending_comp_ver_str,
- pendingCompVerStr, pendingCompVerStrLen));
- EXPECT_EQ('\0',
- entry_version.entry.pending_comp_ver_str[pendingCompVerStrLen]);
- EXPECT_EQ(0, memcmp(entry_version.active_comp_ver_str, activeCompVerStr,
- activeCompVerStrLen));
- EXPECT_EQ('\0', entry_version.active_comp_ver_str[activeCompVerStrLen]);
- EXPECT_EQ(0, memcmp(entry_version.pending_comp_ver_str, pendingCompVerStr,
- pendingCompVerStrLen));
- EXPECT_EQ('\0', entry_version.pending_comp_ver_str[pendingCompVerStrLen]);
-}
-#endif
-
-#ifdef LIBPLDM_API_TESTING
-TEST(GetDownstreamFirmwareParameters, goodPathDecodeDownstreamTableVersions)
-{
- // Arbitrary component version string length
- constexpr uint8_t activeCompVerStrLen = 8;
- constexpr uint8_t pendingCompVerStrLen = 8;
- // Arbitrary ActiveVersionStr and pendingVersionStr
- constexpr char versionsStr[] = {'1', '2', '3', '4', '5', '6', '7', '8',
- 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'};
- const struct variable_field versions = {
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- .ptr = reinterpret_cast<const uint8_t*>(versionsStr),
- .length = sizeof(versionsStr)};
-
- struct pldm_downstream_device_parameter_entry_versions entryVersion = {};
- entryVersion.entry.active_comp_ver_str_len = activeCompVerStrLen;
- entryVersion.entry.pending_comp_ver_str_len = pendingCompVerStrLen;
-
- int rc = decode_downstream_device_parameter_table_entry_versions(
- &versions, &entryVersion.entry, entryVersion.active_comp_ver_str,
- sizeof(entryVersion.active_comp_ver_str),
- entryVersion.pending_comp_ver_str,
- sizeof(entryVersion.pending_comp_ver_str));
-
- EXPECT_EQ(rc, 0);
- EXPECT_EQ(0, memcmp(entryVersion.active_comp_ver_str, versions.ptr,
- activeCompVerStrLen));
- EXPECT_EQ('\0', entryVersion.active_comp_ver_str[activeCompVerStrLen]);
- EXPECT_EQ(0,
- memcmp(entryVersion.pending_comp_ver_str,
- versions.ptr + activeCompVerStrLen, pendingCompVerStrLen));
- EXPECT_EQ('\0', entryVersion.pending_comp_ver_str[activeCompVerStrLen]);
- EXPECT_EQ(0, memcmp(entryVersion.entry.active_comp_ver_str, versions.ptr,
- activeCompVerStrLen));
- EXPECT_EQ('\0',
- entryVersion.entry.pending_comp_ver_str[activeCompVerStrLen]);
- EXPECT_EQ(0,
- memcmp(entryVersion.entry.pending_comp_ver_str,
- versions.ptr + activeCompVerStrLen, pendingCompVerStrLen));
- EXPECT_EQ('\0',
- entryVersion.entry.pending_comp_ver_str[activeCompVerStrLen]);
-}
-#endif
-
-#ifdef LIBPLDM_API_TESTING
-TEST(GetDownstreamFirmwareParameters, decodeInvalidDownstreamTableVersions)
-{
- // Arbitrary ActiveVersionStr and pendingVersionStr
- constexpr char versionsStr[] = {'1', '2', '3', '4', '5', '6', '7', '8',
- 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'};
- const struct variable_field versions = {
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- .ptr = reinterpret_cast<const uint8_t*>(versionsStr),
- .length = sizeof(versionsStr)};
-
- struct pldm_downstream_device_parameter_entry_versions entryVersion = {};
-
- int rc = decode_downstream_device_parameter_table_entry_versions(
- &versions, nullptr, entryVersion.active_comp_ver_str,
- sizeof(entryVersion.active_comp_ver_str),
- entryVersion.pending_comp_ver_str,
- sizeof(entryVersion.pending_comp_ver_str));
- EXPECT_EQ(rc, -EINVAL);
-}
-#endif
-
-#ifdef LIBPLDM_API_TESTING
-TEST(GetDownstreamFirmwareParameters, decodeOverflowDownstreamTableVersions)
-{
- // Arbitrary component version string length
- constexpr uint8_t activeCompVerStrLen = 8;
- constexpr uint8_t pendingCompVerStrLen = 8;
- // Arbitrary ActiveVersionStr and pendingVersionStr
- constexpr char versionsStr[] = {'1', '2', '3', '4', '5', '6', '7', '8',
- 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'};
- const struct variable_field versions = {
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- .ptr = reinterpret_cast<const uint8_t*>(versionsStr),
- .length = sizeof(versionsStr) - 1 // Inject error length
- };
-
- struct pldm_downstream_device_parameter_entry_versions entryVersion = {};
- entryVersion.entry.active_comp_ver_str_len = activeCompVerStrLen;
- entryVersion.entry.pending_comp_ver_str_len = pendingCompVerStrLen;
-
- EXPECT_EQ(decode_downstream_device_parameter_table_entry_versions(
- &versions, &entryVersion.entry,
- entryVersion.active_comp_ver_str,
- sizeof(entryVersion.active_comp_ver_str),
- entryVersion.pending_comp_ver_str,
- sizeof(entryVersion.pending_comp_ver_str)),
- -EOVERFLOW);
+ struct pldm_downstream_device_parameters_entry entry;
+ foreach_pldm_downstream_device_parameters_entry(iter, entry, rc)
+ {
+ FAIL();
+ }
+ EXPECT_EQ(rc, -EOVERFLOW);
}
#endif