platform: pldm_msgbuf for decode_set_numeric_effecter_value_req()
Also, make it at least possible to detect incorrect use of the API by
declaring `effecter_value` as a fixed-size array of 4.
Misuse is detected by -Wstringop-overflow with `gcc`, however failure
to uphold the requirement is ignored by `g++`, and both `clang` and
`clang++`.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I5769db4d3c812d7feef994de836a8621fb4bf3cf
diff --git a/include/libpldm/platform.h b/include/libpldm/platform.h
index 9380bc9..dd4e9b8 100644
--- a/include/libpldm/platform.h
+++ b/include/libpldm/platform.h
@@ -1098,7 +1098,7 @@
size_t payload_length,
uint16_t *effecter_id,
uint8_t *effecter_data_size,
- uint8_t *effecter_value);
+ uint8_t effecter_value[4]);
/** @brief Create a PLDM response message for SetNumericEffecterValue
*
diff --git a/src/msgbuf/platform.h b/src/msgbuf/platform.h
index c10dc45..69403db 100644
--- a/src/msgbuf/platform.h
+++ b/src/msgbuf/platform.h
@@ -73,4 +73,28 @@
return -PLDM_ERROR_INVALID_DATA;
}
+/* This API is bad, but it's because the caller's APIs are also bad */
+__attribute__((always_inline)) static inline int
+pldm_msgbuf_extract_effecter_value(struct pldm_msgbuf *ctx,
+ enum pldm_effecter_data_size tag,
+ uint8_t *val)
+{
+ switch (tag) {
+ case PLDM_EFFECTER_DATA_SIZE_UINT8:
+ return pldm_msgbuf_extract_uint8(ctx, (uint8_t *)val);
+ case PLDM_EFFECTER_DATA_SIZE_SINT8:
+ return pldm_msgbuf_extract_int8(ctx, (int8_t *)val);
+ case PLDM_EFFECTER_DATA_SIZE_UINT16:
+ return pldm_msgbuf_extract_uint16(ctx, (uint16_t *)val);
+ case PLDM_EFFECTER_DATA_SIZE_SINT16:
+ return pldm_msgbuf_extract_int16(ctx, (int16_t *)val);
+ case PLDM_EFFECTER_DATA_SIZE_UINT32:
+ return pldm_msgbuf_extract_uint32(ctx, (uint32_t *)val);
+ case PLDM_EFFECTER_DATA_SIZE_SINT32:
+ return pldm_msgbuf_extract_int32(ctx, (int32_t *)val);
+ }
+
+ return -PLDM_ERROR_INVALID_DATA;
+}
+
#endif
diff --git a/src/platform.c b/src/platform.c
index 25c18bc..de07c2c 100644
--- a/src/platform.c
+++ b/src/platform.c
@@ -536,64 +536,38 @@
size_t payload_length,
uint16_t *effecter_id,
uint8_t *effecter_data_size,
- uint8_t *effecter_value)
+ uint8_t effecter_value[4])
{
+ struct pldm_msgbuf _buf;
+ struct pldm_msgbuf *buf = &_buf;
+ int rc;
+
if (msg == NULL || effecter_id == NULL || effecter_data_size == NULL ||
effecter_value == NULL) {
return PLDM_ERROR_INVALID_DATA;
}
- if (payload_length < PLDM_SET_NUMERIC_EFFECTER_VALUE_MIN_REQ_BYTES) {
- return PLDM_ERROR_INVALID_LENGTH;
+ rc =
+ pldm_msgbuf_init(buf, PLDM_SET_NUMERIC_EFFECTER_VALUE_MIN_REQ_BYTES,
+ msg->payload, payload_length);
+ if (rc) {
+ return rc;
}
- struct pldm_set_numeric_effecter_value_req *request =
- (struct pldm_set_numeric_effecter_value_req *)msg->payload;
- *effecter_id = le16toh(request->effecter_id);
- *effecter_data_size = request->effecter_data_size;
+ pldm_msgbuf_extract(buf, effecter_id);
+ rc = pldm_msgbuf_extract(buf, effecter_data_size);
+ if (rc) {
+ return PLDM_ERROR_INVALID_DATA;
+ }
if (*effecter_data_size > PLDM_EFFECTER_DATA_SIZE_SINT32) {
return PLDM_ERROR_INVALID_DATA;
}
- if (*effecter_data_size == PLDM_EFFECTER_DATA_SIZE_UINT8 ||
- *effecter_data_size == PLDM_EFFECTER_DATA_SIZE_SINT8) {
+ pldm_msgbuf_extract_effecter_value(buf, *effecter_data_size,
+ effecter_value);
- if (payload_length !=
- PLDM_SET_NUMERIC_EFFECTER_VALUE_MIN_REQ_BYTES) {
- return PLDM_ERROR_INVALID_LENGTH;
- }
-
- *effecter_value = request->effecter_value[0];
- }
-
- if (*effecter_data_size == PLDM_EFFECTER_DATA_SIZE_UINT16 ||
- *effecter_data_size == PLDM_EFFECTER_DATA_SIZE_SINT16) {
-
- if (payload_length !=
- PLDM_SET_NUMERIC_EFFECTER_VALUE_MIN_REQ_BYTES + 1) {
- return PLDM_ERROR_INVALID_LENGTH;
- }
-
- memcpy(effecter_value, request->effecter_value, 2);
- uint16_t *val = (uint16_t *)(effecter_value);
- *val = le16toh(*val);
- }
-
- if (*effecter_data_size == PLDM_EFFECTER_DATA_SIZE_UINT32 ||
- *effecter_data_size == PLDM_EFFECTER_DATA_SIZE_SINT32) {
-
- if (payload_length !=
- PLDM_SET_NUMERIC_EFFECTER_VALUE_MIN_REQ_BYTES + 3) {
- return PLDM_ERROR_INVALID_LENGTH;
- }
-
- memcpy(effecter_value, request->effecter_value, 4);
- uint32_t *val = (uint32_t *)(effecter_value);
- *val = le32toh(*val);
- }
-
- return PLDM_SUCCESS;
+ return pldm_msgbuf_destroy(buf);
}
int encode_set_numeric_effecter_value_resp(uint8_t instance_id,