tran_sock: use ERROR_INT() (#431)

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>

3 files changed, 100 insertions, 90 deletions

diff --git a/lib/irq.c b/lib/irq.c
index 25eb31b..a274191 100644
--- a/lib/irq.c
+++ b/lib/irq.c
@@ -37,7 +37,6 @@
 #include <sys/eventfd.h>
 
 #include "irq.h"
-#include "tran_sock.h"
 
 #define LM2VFIO_IRQT(type) (type - 1)
 
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index c86713a..47d0651 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -697,8 +697,8 @@ validate_header(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size)
  * Populates @hdr to contain the header for the next command to be processed.
  * Stores any passed FDs into @fds and the number in @nr_fds.
  *
- * Returns 0 if there is no command to process, -errno if an error occured, or
- * the number of bytes read.
+ * Returns 0 if there is no command to process, -errno on error, or the number
+ * of bytes read.
  */
 int
 MOCK_DEFINE(get_next_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
@@ -706,10 +706,9 @@ MOCK_DEFINE(get_next_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
 {
     int ret;
 
-    /* FIXME get request shouldn't set errno, it should return it as -errno */
     ret = vfu_ctx->tran->get_request(vfu_ctx, hdr, fds, nr_fds);
     if (unlikely(ret < 0)) {
-        switch (-ret) {
+        switch (errno) {
         case EAGAIN:
             return 0;
 
@@ -722,9 +721,8 @@ MOCK_DEFINE(get_next_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
             return -ENOTCONN;
 
         default:
-            vfu_log(vfu_ctx, LOG_ERR, "failed to receive request: %s",
-                   strerror(-ret));
-            return ret;
+            vfu_log(vfu_ctx, LOG_ERR, "failed to receive request: %m");
+            return -errno;
         }
     }
 
@@ -794,14 +792,16 @@ MOCK_DEFINE(exec_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
     if (cmd_data_size > 0) {
         ret = vfu_ctx->tran->recv_body(vfu_ctx, hdr, &cmd_data);
 
-        if (ret == -ENOMSG) {
-            vfu_reset_ctx(vfu_ctx, "closed");
-            return -ENOTCONN;
-        } else if (ret == -ECONNRESET) {
-            vfu_reset_ctx(vfu_ctx, "reset");
-            return -ENOTCONN;
-        } else if (ret < 0) {
-            return ret;
+        if (ret < 0) {
+            if (errno == ENOMSG) {
+                vfu_reset_ctx(vfu_ctx, "closed");
+                return -ENOTCONN;
+            } else if (errno == ECONNRESET) {
+                vfu_reset_ctx(vfu_ctx, "reset");
+                return -ENOTCONN;
+            } else {
+                return -errno;
+            }
         }
     }
 
@@ -981,14 +981,16 @@ MOCK_DEFINE(process_request)(vfu_ctx_t *vfu_ctx)
                                    fds_out, nr_fds_out, -ret);
 
         if (ret < 0) {
-            vfu_log(vfu_ctx, LOG_ERR, "failed to reply: %s", strerror(-ret));
+            vfu_log(vfu_ctx, LOG_ERR, "failed to reply: %m");
 
-            if (ret == -ECONNRESET) {
+            if (errno == ECONNRESET) {
                 vfu_reset_ctx(vfu_ctx, "reset");
                 ret = -ENOTCONN;
-            } else if (ret == -ENOMSG) {
+            } else if (errno == ENOMSG) {
                 vfu_reset_ctx(vfu_ctx, "closed");
                 ret = -ENOTCONN;
+            } else {
+                ret = -errno;
             }
         }
     }
@@ -1239,6 +1241,7 @@ vfu_create_ctx(vfu_trans_t trans, const char *path, int flags, void *pvt,
     if (vfu_ctx->tran->init != NULL) {
         err = vfu_ctx->tran->init(vfu_ctx);
         if (err < 0) {
+            err = -errno;
             goto err_out;
         }
     }
@@ -1564,7 +1567,7 @@ vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
 
     dma_recv = calloc(recv_size, 1);
     if (dma_recv == NULL) {
-        return ERROR_INT(ENOMEM);
+        return -1;
     }
 
     dma_send.addr = (uint64_t)sg->dma_addr;
@@ -1573,20 +1576,22 @@ vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
                                   &dma_send, sizeof(dma_send), NULL,
                                   dma_recv, recv_size);
 
-    if (ret == -ENOMSG) {
-        vfu_reset_ctx(vfu_ctx, "closed");
-        ret = -ENOTCONN;
-    } else if (ret == -ECONNRESET) {
-        vfu_reset_ctx(vfu_ctx, "reset");
-        ret = -ENOTCONN;
-    } else if (ret == 0) {
+    if (ret < 0) {
+        if (errno == ENOMSG) {
+            vfu_reset_ctx(vfu_ctx, "closed");
+            errno = ENOTCONN;
+        } else if (errno == ECONNRESET) {
+            vfu_reset_ctx(vfu_ctx, "reset");
+            errno = ENOTCONN;
+        }
+    } else {
         /* FIXME no need for memcpy */
         memcpy(data, dma_recv->data, sg->length);
     }
 
     free(dma_recv);
 
-    return ret < 0 ? ERROR_INT(-ret) : 0;
+    return ret;
 }
 
 int
@@ -1601,7 +1606,7 @@ vfu_dma_write(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
 
     dma_send = calloc(send_size, 1);
     if (dma_send == NULL) {
-        return ERROR_INT(ENOMEM);
+        return -1;
     }
     dma_send->addr = (uint64_t)sg->dma_addr;
     dma_send->count = sg->length;
@@ -1610,17 +1615,19 @@ vfu_dma_write(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
                                   dma_send, send_size, NULL,
                                   &dma_recv, sizeof(dma_recv));
 
-    if (ret == -ENOMSG) {
-        vfu_reset_ctx(vfu_ctx, "closed");
-        ret = -ENOTCONN;
-    } else if (ret == -ECONNRESET) {
-        vfu_reset_ctx(vfu_ctx, "reset");
-        ret = -ENOTCONN;
+    if (ret < 0) {
+        if (errno == ENOMSG) {
+            vfu_reset_ctx(vfu_ctx, "closed");
+            errno = ENOTCONN;
+        } else if (errno == ECONNRESET) {
+            vfu_reset_ctx(vfu_ctx, "reset");
+            errno = ENOTCONN;
+        }
     }
 
     free(dma_send);
 
-    return ret < 0 ? ERROR_INT(-ret) : 0;
+    return ret;
 }
 
 uint64_t
diff --git a/lib/tran_sock.c b/lib/tran_sock.c
index a1d6a1a..868a5be 100644
--- a/lib/tran_sock.c
+++ b/lib/tran_sock.c
@@ -121,11 +121,11 @@ MOCK_DEFINE(tran_sock_send_iovec)(int sock, uint16_t msg_id, bool is_reply,
     if (ret == -1) {
         /* Treat a failed write due to EPIPE the same as a short write. */
         if (errno == EPIPE) {
-            return -ECONNRESET;
+            return ERROR_INT(ECONNRESET);
         }
-        return -errno;
+        return -1;
     } else if ((size_t)ret < hdr.msg_size) {
-        return -ECONNRESET;
+        return ERROR_INT(ECONNRESET);
     }
 
     return 0;
@@ -177,15 +177,15 @@ get_msg(void *data, size_t len, int *fds, size_t *nr_fds, int sock_fd,
 
     ret = recvmsg(sock_fd, &msg, sock_flags);
     if (ret == -1) {
-        return -errno;
+        return -1;
     } else if (ret == 0) {
-        return -ENOMSG;
+        return ERROR_INT(ENOMSG);
     } else if ((size_t)ret < len) {
-        return -ECONNRESET;
+        return ERROR_INT(ECONNRESET);
     }
 
     if (msg.msg_flags & MSG_CTRUNC || msg.msg_flags & MSG_TRUNC) {
-        return -EFAULT;
+        return ERROR_INT(EFAULT);
     }
 
     if (nr_fds != NULL) {
@@ -194,11 +194,11 @@ get_msg(void *data, size_t len, int *fds, size_t *nr_fds, int sock_fd,
                 continue;
             }
             if (cmsg->cmsg_len < CMSG_LEN(sizeof(int))) {
-                return -EINVAL;
+                return ERROR_INT(EINVAL);
             }
             int size = cmsg->cmsg_len - CMSG_LEN(0);
             if (size % sizeof(int) != 0) {
-                return -EINVAL;
+                return ERROR_INT(EINVAL);
             }
             *nr_fds = (int)(size / sizeof(int));
             memcpy(fds, CMSG_DATA(cmsg), *nr_fds * sizeof(int));
@@ -234,22 +234,22 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply,
 
     if (is_reply) {
         if (msg_id != NULL && hdr->msg_id != *msg_id) {
-            return -EPROTO;
+            return ERROR_INT(EPROTO);
         }
 
         if (hdr->flags.type != VFIO_USER_F_TYPE_REPLY) {
-            return -EINVAL;
+            return ERROR_INT(EINVAL);
         }
 
         if (hdr->flags.error == 1U) {
             if (hdr->error_no <= 0) {
                 hdr->error_no = EINVAL;
             }
-            return -hdr->error_no;
+            return ERROR_INT(hdr->error_no);
         }
     } else {
         if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) {
-            return -EINVAL;
+            return ERROR_INT(EINVAL);
         }
         if (msg_id != NULL) {
             *msg_id = hdr->msg_id;
@@ -260,11 +260,11 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply,
         ret = recv(sock, data, MIN(hdr->msg_size - sizeof(*hdr), *len),
                    MSG_WAITALL);
         if (ret < 0) {
-            return -errno;
+            return -1;
         } else if (ret == 0) {
-            return -ENOMSG;
+            return ERROR_INT(ENOMSG);
         } else if (*len != (size_t)ret) {
-            return -ECONNRESET;
+            return ERROR_INT(ECONNRESET);
         }
         *len = ret;
     }
@@ -312,20 +312,20 @@ tran_sock_recv_alloc(int sock, struct vfio_user_header *hdr, bool is_reply,
     data = calloc(1, len);
 
     if (data == NULL) {
-        return -errno;
+        return -1;
     }
 
     ret = recv(sock, data, len, MSG_WAITALL);
     if (ret < 0) {
-        ret = -errno;
+        ret = errno;
         free(data);
-        return ret;
+        return ERROR_INT(ret);
     } else if (ret == 0) {
         free(data);
-        return -ENOMSG;
+        return ERROR_INT(ENOMSG);
     } else if (len != (size_t)ret) {
         free(data);
-        return -ECONNRESET;
+        return ERROR_INT(ECONNRESET);
     }
 
     *datap = data;
@@ -398,7 +398,7 @@ tran_sock_init(vfu_ctx_t *vfu_ctx)
     ts = calloc(1, sizeof(tran_sock_t));
 
     if (ts == NULL) {
-        ret = -errno;
+        ret = errno;
         goto out;
     }
 
@@ -406,7 +406,7 @@ tran_sock_init(vfu_ctx_t *vfu_ctx)
     ts->conn_fd = -1;
 
     if ((ts->listen_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
-        ret = -errno;
+        ret = errno;
         goto out;
     }
 
@@ -414,29 +414,29 @@ tran_sock_init(vfu_ctx_t *vfu_ctx)
         ret = fcntl(ts->listen_fd, F_SETFL,
                     fcntl(ts->listen_fd, F_GETFL, 0) | O_NONBLOCK);
         if (ret < 0) {
-            ret = -errno;
+            ret = errno;
             goto out;
         }
     }
 
     ret = snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", vfu_ctx->uuid);
     if (ret >= (int)sizeof(addr.sun_path)) {
-        ret = -ENAMETOOLONG;
-    }
-    if (ret < 0) {
+        ret = ENAMETOOLONG;
+    } else if (ret < 0) {
+        ret = EINVAL;
         goto out;
     }
 
     /* start listening business */
     ret = bind(ts->listen_fd, (struct sockaddr *)&addr, sizeof(addr));
     if (ret < 0) {
-        ret = -errno;
+        ret = errno;
         goto out;
     }
 
     ret = listen(ts->listen_fd, 0);
     if (ret < 0) {
-        ret = -errno;
+        ret = errno;
     }
 
 out:
@@ -445,7 +445,7 @@ out:
             close(ts->listen_fd);
         }
         free(ts);
-        return ret;
+        return ERROR_INT(ret);
     }
 
     vfu_ctx->tran_data = ts;
@@ -486,7 +486,7 @@ tran_parse_version_json(const char *json_str,
     struct json_object *jo_caps = NULL;
     struct json_object *jo_top = NULL;
     struct json_object *jo = NULL;
-    int ret = -EINVAL;
+    int ret = EINVAL;
 
     if ((jo_top = json_tokener_parse(json_str)) == NULL) {
         goto out;
@@ -540,7 +540,10 @@ tran_parse_version_json(const char *json_str,
 out:
     /* We just need to put our top-level object. */
     json_object_put(jo_top);
-    return ret;
+    if (ret != 0) {
+        return ERROR_INT(ret);
+    }
+    return 0;
 }
 
 static int
@@ -558,29 +561,28 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp,
                                (void **)&cversion, &vlen);
 
     if (ret < 0) {
-        vfu_log(vfu_ctx, LOG_ERR, "failed to receive version: %s",
-                strerror(-ret));
+        vfu_log(vfu_ctx, LOG_ERR, "failed to receive version: %m");
         return ret;
     }
 
     if (hdr.cmd != VFIO_USER_VERSION) {
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: invalid cmd %hu (expected %u)",
                 *msg_idp, hdr.cmd, VFIO_USER_VERSION);
-        ret = -EINVAL;
+        ret = EINVAL;
         goto out;
     }
 
     if (vlen < sizeof(*cversion)) {
         vfu_log(vfu_ctx, LOG_ERR,
                 "msg%#hx: VFIO_USER_VERSION: invalid size %lu", *msg_idp, vlen);
-        ret = -EINVAL;
+        ret = EINVAL;
         goto out;
     }
 
     if (cversion->major != LIB_VFIO_USER_MAJOR) {
         vfu_log(vfu_ctx, LOG_ERR, "unsupported client major %hu (must be %u)",
                 cversion->major, LIB_VFIO_USER_MAJOR);
-        ret = -ENOTSUP;
+        ret = EINVAL;
         goto out;
     }
 
@@ -593,7 +595,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp,
 
         if (json_str[len - 1] != '\0') {
             vfu_log(vfu_ctx, LOG_ERR, "ignoring invalid JSON from client");
-            ret = -EINVAL;
+            ret = EINVAL;
             goto out;
         }
 
@@ -608,6 +610,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp,
 #else
             vfu_log(vfu_ctx, LOG_ERR, "failed to parse client JSON");
 #endif
+            ret = errno;
             goto out;
         }
 
@@ -617,6 +620,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp,
             if (ret != 0) {
                 vfu_log(vfu_ctx, LOG_ERR, "refusing client page size of %zu",
                         pgsize);
+                ret = -ret;
                 goto out;
             }
         }
@@ -626,7 +630,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp,
             vfu_ctx->client_max_fds > VFIO_USER_CLIENT_MAX_FDS_LIMIT) {
             vfu_log(vfu_ctx, LOG_ERR, "refusing client max_fds of %d",
                     vfu_ctx->client_max_fds);
-            ret = -EINVAL;
+            ret = EINVAL;
             goto out;
         }
     }
@@ -636,11 +640,12 @@ out:
         // FIXME: spec, is it OK to just have the header?
         (void) tran_sock_send_error(sock, *msg_idp, hdr.cmd, ret);
         free(cversion);
-        cversion = NULL;
+        *versionp = NULL;
+        return ERROR_INT(ret);
     }
 
     *versionp = cversion;
-    return ret;
+    return 0;
 }
 
 static int
@@ -700,7 +705,7 @@ negotiate(vfu_ctx_t *vfu_ctx, int sock)
     ret = recv_version(vfu_ctx, sock, &msg_id, &client_version);
 
     if (ret < 0) {
-        vfu_log(vfu_ctx, LOG_ERR, "failed to recv version: %s", strerror(-ret));
+        vfu_log(vfu_ctx, LOG_ERR, "failed to recv version: %m");
         return ret;
     }
 
@@ -709,7 +714,7 @@ negotiate(vfu_ctx_t *vfu_ctx, int sock)
     free(client_version);
 
     if (ret < 0) {
-        vfu_log(vfu_ctx, LOG_ERR, "failed to send version: %s", strerror(-ret));
+        vfu_log(vfu_ctx, LOG_ERR, "failed to send version: %m");
     }
 
     return ret;
@@ -729,8 +734,7 @@ tran_sock_attach(vfu_ctx_t *vfu_ctx)
     if (ts->conn_fd != -1) {
         vfu_log(vfu_ctx, LOG_ERR, "%s: already attached with fd=%d",
                 __func__, ts->conn_fd);
-        errno = EINVAL;
-        return -1;
+        return ERROR_INT(EINVAL);
     }
 
     ts->conn_fd = accept(ts->listen_fd, NULL, NULL);
@@ -740,10 +744,10 @@ tran_sock_attach(vfu_ctx_t *vfu_ctx)
 
     ret = negotiate(vfu_ctx, ts->conn_fd);
     if (ret < 0) {
+        ret = errno;
         close(ts->conn_fd);
         ts->conn_fd = -1;
-        errno = -ret;
-        return -1;
+        return ERROR_INT(ret);
     }
 
     return 0;
@@ -763,7 +767,7 @@ tran_sock_get_request(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
 
     if (ts->conn_fd == -1) {
         vfu_log(vfu_ctx, LOG_ERR, "%s: not connected", __func__);
-        return -ENOTCONN;
+        return ERROR_INT(ENOTCONN);
     }
 
     /*
@@ -793,7 +797,7 @@ tran_sock_recv_body(vfu_ctx_t *vfu_ctx, const struct vfio_user_header *hdr,
     if (hdr->msg_size > SERVER_MAX_MSG_SIZE) {
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: size of %u is too large",
                 hdr->msg_id, hdr->msg_size);
-        return -EINVAL;
+        return ERROR_INT(EINVAL);
     }
 
     ts = vfu_ctx->tran_data;
@@ -803,23 +807,23 @@ tran_sock_recv_body(vfu_ctx_t *vfu_ctx, const struct vfio_user_header *hdr,
     data = malloc(body_size);
 
     if (data == NULL) {
-        return -errno;
+        return -1;
     }
 
     ret = recv(ts->conn_fd, data, body_size, 0);
 
     if (ret < 0) {
-        ret = -errno;
+        ret = errno;
         free(data);
-        return ret;
+        return ERROR_INT(ret);
     } else if (ret == 0) {
         free(data);
-        return -ENOMSG;
+        return ERROR_INT(ENOMSG);
     } else if (ret != (int)body_size) {
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: short read: expected=%zu, actual=%d",
                 hdr->msg_id, body_size, ret);
         free(data);
-        return -ECONNRESET;
+        return ERROR_INT(ECONNRESET);
     }
 
     *datap = data;