dsp: base: Rework {un,}pack_pldm_header() error handling
The current preference is that library APIs return negative errno values
to signal implementation errors. That doesn't jive with existing stable
APIs returning PLDM completion codes, so provide a means to translate
between the two.
The first users are the {un,}pack_pldm_header() functions.
Change-Id: I7b7cb97a1d8b96ec0fec1c0a5fbd8503da834d86
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/src/api.h b/src/api.h
new file mode 100644
index 0000000..3f1fed0
--- /dev/null
+++ b/src/api.h
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+#ifndef LIBPLDM_SRC_API_H
+#define LIBPLDM_SRC_API_H
+
+#include <libpldm/base.h>
+
+#include <assert.h>
+#include <errno.h>
+
+/**
+ * @brief Translate a negative errno value to a PLDM completion code
+ *
+ * Existing stable APIs often return errors in the form of PLDM completion
+ * codes, which confuses the problems of the protocol with the problems of
+ * the implementation. We're shifting to using negative errno values to signal
+ * implementation errors. However, for existing stable APIs, provide a means to
+ * translate between the two.
+ *
+ * @param[in] err - The negative errno to translate to a completion code
+ *
+ * @return An equivalent PLDM completion code for @p err
+ */
+static inline enum pldm_completion_codes pldm_xlate_errno(int err)
+{
+ enum pldm_completion_codes rc;
+
+ assert(err < 0);
+ switch (err) {
+ case -EINVAL:
+ rc = PLDM_ERROR_INVALID_DATA;
+ break;
+ case -ENOMSG:
+ rc = PLDM_ERROR_INVALID_PLDM_TYPE;
+ break;
+ default:
+ assert(false);
+ rc = PLDM_ERROR;
+ break;
+ }
+
+ assert(rc > 0);
+ return rc;
+}
+
+#endif
diff --git a/src/dsp/base.c b/src/dsp/base.c
index eec7626..7aca3c8 100644
--- a/src/dsp/base.c
+++ b/src/dsp/base.c
@@ -1,30 +1,34 @@
/* SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later */
+#include "api.h"
+#include "dsp/base.h"
+
+#include <assert.h>
#include <libpldm/base.h>
#include <libpldm/pldm_types.h>
#include <endian.h>
+#include <errno.h>
#include <stdint.h>
#include <string.h>
-LIBPLDM_ABI_STABLE
-uint8_t pack_pldm_header(const struct pldm_header_info *hdr,
- struct pldm_msg_hdr *msg)
+int pack_pldm_header_errno(const struct pldm_header_info *hdr,
+ struct pldm_msg_hdr *msg)
{
if (msg == NULL || hdr == NULL) {
- return PLDM_ERROR_INVALID_DATA;
+ return -EINVAL;
}
if (hdr->msg_type != PLDM_RESPONSE && hdr->msg_type != PLDM_REQUEST &&
hdr->msg_type != PLDM_ASYNC_REQUEST_NOTIFY) {
- return PLDM_ERROR_INVALID_DATA;
+ return -EINVAL;
}
if (hdr->instance > PLDM_INSTANCE_MAX) {
- return PLDM_ERROR_INVALID_DATA;
+ return -EINVAL;
}
if (hdr->pldm_type > (PLDM_MAX_TYPES - 1)) {
- return PLDM_ERROR_INVALID_PLDM_TYPE;
+ return -ENOMSG;
}
uint8_t datagram = (hdr->msg_type == PLDM_ASYNC_REQUEST_NOTIFY) ? 1 : 0;
@@ -42,15 +46,14 @@
msg->type = hdr->pldm_type;
msg->command = hdr->command;
- return PLDM_SUCCESS;
+ return 0;
}
-LIBPLDM_ABI_STABLE
-uint8_t unpack_pldm_header(const struct pldm_msg_hdr *msg,
- struct pldm_header_info *hdr)
+int unpack_pldm_header_errno(const struct pldm_msg_hdr *msg,
+ struct pldm_header_info *hdr)
{
if (msg == NULL) {
- return PLDM_ERROR_INVALID_DATA;
+ return -EINVAL;
}
if (msg->request == PLDM_RESPONSE) {
@@ -64,7 +67,51 @@
hdr->pldm_type = msg->type;
hdr->command = msg->command;
- return PLDM_SUCCESS;
+ return 0;
+}
+
+LIBPLDM_ABI_STABLE
+uint8_t pack_pldm_header(const struct pldm_header_info *hdr,
+ struct pldm_msg_hdr *msg)
+{
+ enum pldm_completion_codes cc;
+ int rc;
+
+ rc = pack_pldm_header_errno(hdr, msg);
+ if (!rc) {
+ return PLDM_SUCCESS;
+ }
+
+ cc = pldm_xlate_errno(rc);
+ assert(cc < UINT8_MAX);
+ if (cc > UINT8_MAX) {
+ static_assert(PLDM_ERROR < UINT8_MAX, "Unable to report error");
+ return PLDM_ERROR;
+ }
+
+ return cc;
+}
+
+LIBPLDM_ABI_STABLE
+uint8_t unpack_pldm_header(const struct pldm_msg_hdr *msg,
+ struct pldm_header_info *hdr)
+{
+ enum pldm_completion_codes cc;
+ int rc;
+
+ rc = unpack_pldm_header_errno(msg, hdr);
+ if (!rc) {
+ return PLDM_SUCCESS;
+ }
+
+ cc = pldm_xlate_errno(rc);
+ assert(cc < UINT8_MAX);
+ if (cc > UINT8_MAX) {
+ static_assert(PLDM_ERROR < UINT8_MAX, "Unable to report error");
+ return PLDM_ERROR;
+ }
+
+ return cc;
}
LIBPLDM_ABI_STABLE
diff --git a/src/dsp/base.h b/src/dsp/base.h
new file mode 100644
index 0000000..0e2c1a0
--- /dev/null
+++ b/src/dsp/base.h
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+#ifndef LIBPLDM_SRC_DSP_BASE_H
+#define LIBPLDM_SRC_DSP_BASE_H
+
+/* Internal functions */
+
+#include <libpldm/base.h>
+
+int pack_pldm_header_errno(const struct pldm_header_info *hdr,
+ struct pldm_msg_hdr *msg);
+
+int unpack_pldm_header_errno(const struct pldm_msg_hdr *msg,
+ struct pldm_header_info *hdr);
+
+#endif
diff --git a/src/msgbuf.h b/src/msgbuf.h
index f69c428..3dcfa42 100644
--- a/src/msgbuf.h
+++ b/src/msgbuf.h
@@ -80,6 +80,21 @@
enum pldm_msgbuf_error_mode mode;
};
+/**
+ * @brief Either negate an errno value or return a value mapped to a PLDM
+ * completion code.
+ *
+ * Note that `pldm_msgbuf_status()` is purely internal to the msgbuf API
+ * for ergonomics. It's preferred that we don't try to unify this with
+ * `pldm_xlate_errno()` from src/api.h despite the similarities.
+ *
+ * @param[in] ctx - The msgbuf context providing the personality info
+ * @param[in] err - The positive errno value to translate
+ *
+ * @return Either the negated value of @p err if the context's error mode is
+ * `PLDM_MSGBUF_C_ERRNO`, or the equivalent PLDM completion code if the
+ * error mode is `PLDM_MSGBUF_PLDM_CC`.
+ */
__attribute__((always_inline)) static inline int
pldm_msgbuf_status(struct pldm_msgbuf *ctx, unsigned int err)
{