requester: Return PLDM_REQUESTER_OPEN_FAIL from pldm_open() on error
As it stood the reimplementation of pldm_open() passed back the return
value of the pldm_transport_mctp_demux_*() APIs, which don't align with
the specified behaviour of pldm_open()'s return values.
Rework the return values such that PLDM_REQUESTER_OPEN_FAIL is always
returned on error. This fixes error handling in at least
openpower-occ-control, which only tested for that value and considered
all other values as success.
Further, handle any external close(2) of the returned file descriptor.
This again caters to openpower-occ-control which issues close() in its
response handler.
Fixes: 39f883259956 ("requester: Make pldm_open() return existing fd")
Fixes: c1b66f420912 ("requester: Add new APIs to support multiple transports")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I7144f6ecf0fdfbbc3a2a418a651207c012e0db54
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf93e66..a3c6b49 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,7 @@
1. pdr: Return success for pldm_pdr_find_child_container_id_range_exclude() API
2. pdr: Rework pldm_pdr_find_container_id_range_exclude() API
3. transport: mctp-demux: Don't test socket for non-zero value
+4. requester: Return PLDM_REQUESTER_OPEN_FAIL from pldm_open() on error
## [0.3.0] - 2023-06-23
diff --git a/src/requester/pldm.c b/src/requester/pldm.c
index 8fd5d38..0c28b31 100644
--- a/src/requester/pldm.c
+++ b/src/requester/pldm.c
@@ -3,6 +3,7 @@
#include "libpldm/transport.h"
#include <bits/types/struct_iovec.h>
+#include <fcntl.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
@@ -28,24 +29,31 @@
LIBPLDM_ABI_STABLE
pldm_requester_rc_t pldm_open(void)
{
- int fd;
- int rc;
+ int fd = PLDM_REQUESTER_OPEN_FAIL;
if (open_transport) {
fd = pldm_transport_mctp_demux_get_socket_fd(open_transport);
- return fd;
+
+ /* If someone has externally issued close() on fd then we need to start again. Use
+ * `fcntl(..., F_GETFD)` to test whether fd is valid. */
+ if (fd < 0 || fcntl(fd, F_GETFD) < 0) {
+ pldm_close();
+ }
}
- struct pldm_transport_mctp_demux *demux = NULL;
- rc = pldm_transport_mctp_demux_init(&demux);
- if (rc) {
- return rc;
+ /* We retest open_transport as it may have been set to NULL by pldm_close() above. */
+ if (!open_transport) {
+ struct pldm_transport_mctp_demux *demux = NULL;
+
+ if (pldm_transport_mctp_demux_init(&demux) < 0) {
+ return PLDM_REQUESTER_OPEN_FAIL;
+ }
+
+ open_transport = demux;
+
+ fd = pldm_transport_mctp_demux_get_socket_fd(open_transport);
}
- fd = pldm_transport_mctp_demux_get_socket_fd(demux);
-
- open_transport = demux;
-
return fd;
}