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: 39f8832 ("requester: Make pldm_open() return existing fd")
Fixes: c1b66f4 ("requester: Add new APIs to support multiple transports")
Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I7144f6ecf0fdfbbc3a2a418a651207c012e0db54

CHANGELOG.md

@@ -40,6 +40,7 @@ Change categories:
 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
 

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,23 +29,30 @@ static struct pldm_transport_mctp_demux *open_transport;
 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;
-	}
 
-	struct pldm_transport_mctp_demux *demux = NULL;
-	rc = pldm_transport_mctp_demux_init(&demux);
-	if (rc) {
-		return rc;
+		/* 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();
+		}
 	}
 
-	fd = pldm_transport_mctp_demux_get_socket_fd(demux);
+	/* 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;
+		open_transport = demux;
+
+		fd = pldm_transport_mctp_demux_get_socket_fd(open_transport);
+	}
 
 	return fd;
 }