API error return converged to one func (#325)

* API error return converged to one func

Use ERROR_INT() or ERROR_PTR() to return errors from API's.
This way if we want to change the behaviour later, we will just need
to update these funcitons.
Also fixed some error return cases and comments.

Reviewed-by: John Levon <john.levon@nutanix.com>

6 files changed, 71 insertions, 68 deletions

diff --git a/include/libvfio-user.h b/include/libvfio-user.h
index 1cc62d9..62d4e77 100644
--- a/include/libvfio-user.h
+++ b/include/libvfio-user.h
@@ -546,7 +546,7 @@ vfu_addr_to_sg(vfu_ctx_t *vfu_ctx, dma_addr_t dma_addr, uint32_t len,
  *       mapping
  * @cnt: number of scatter/gather entries to map
  *
- * @returns 0 on success, -1 on failure
+ * @returns 0 on success, -1 on failure. Sets errno.
  */
 int
 vfu_map_sg(vfu_ctx_t *vfu_ctx, const dma_sg_t *sg,
@@ -573,6 +573,8 @@ vfu_unmap_sg(vfu_ctx_t *vfu_ctx, const dma_sg_t *sg,
  * @vfu_ctx: the libvfio-user context
  * @sg: a DMA segment obtained from dma_addr_to_sg
  * @data: data buffer to read into
+ *
+ * @returns 0 on success, -1 on failure. Sets errno.
  */
 int
 vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data);
@@ -583,6 +585,8 @@ vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data);
  * @vfu_ctx: the libvfio-user context
  * @sg: a DMA segment obtained from dma_addr_to_sg
  * @data: data buffer to write
+ *
+ * @returns 0 on success, -1 on failure. Sets errno.
  */
 int
 vfu_dma_write(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data);
diff --git a/lib/irq.c b/lib/irq.c
index 5c7a1e4..4cc812b 100644
--- a/lib/irq.c
+++ b/lib/irq.c
@@ -376,41 +376,34 @@ handle_device_set_irqs(vfu_ctx_t *vfu_ctx, uint32_t size,
     return dev_set_irqs(vfu_ctx, irq_set, data);
 }
 
-static int
+static bool
 validate_irq_subindex(vfu_ctx_t *vfu_ctx, uint32_t subindex)
 {
     if (vfu_ctx == NULL) {
-        errno = EINVAL;
-        return -1;
+        return false;
     }
 
     if ((subindex >= vfu_ctx->irqs->max_ivs)) {
         vfu_log(vfu_ctx, LOG_ERR, "bad IRQ %d, max=%d\n", subindex,
                vfu_ctx->irqs->max_ivs);
-        /* FIXME should return -errno */
-        errno = EINVAL;
-        return -1;
+        return false;
     }
 
-    return 0;
+    return true;
 }
 
 int
 vfu_irq_trigger(vfu_ctx_t *vfu_ctx, uint32_t subindex)
 {
-    int ret;
     eventfd_t val = 1;
 
-    ret = validate_irq_subindex(vfu_ctx, subindex);
-    if (ret < 0) {
-        return ret;
+    if (!validate_irq_subindex(vfu_ctx, subindex)) {
+        return ERROR_INT(EINVAL);
     }
 
     if (vfu_ctx->irqs->efds[subindex] == -1) {
         vfu_log(vfu_ctx, LOG_ERR, "no fd for interrupt %d\n", subindex);
-        /* FIXME should return -errno */
-        errno = ENOENT;
-        return -1;
+        return ERROR_INT(ENOENT);
     }
 
     return eventfd_write(vfu_ctx->irqs->efds[subindex], val);
@@ -422,9 +415,8 @@ vfu_irq_message(vfu_ctx_t *vfu_ctx, uint32_t subindex)
     int ret, msg_id = 1;
     struct vfio_user_irq_info irq_info;
 
-    ret = validate_irq_subindex(vfu_ctx, subindex);
-    if (ret < 0) {
-        return -1;
+    if (!validate_irq_subindex(vfu_ctx, subindex)) {
+        return ERROR_INT(EINVAL);
     }
 
     irq_info.subindex = subindex;
@@ -433,9 +425,7 @@ vfu_irq_message(vfu_ctx_t *vfu_ctx, uint32_t subindex)
                                    &irq_info, sizeof irq_info,
                                    NULL, NULL, 0);
     if (ret < 0) {
-        /* FIXME should return -errno */
-	    errno = -ret;
-	    return -1;
+	    return ERROR_INT(-ret);
     }
 
     return 0;
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index e5ba0aa..fc427f5 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -986,7 +986,7 @@ vfu_realize_ctx(vfu_ctx_t *vfu_ctx)
     if (vfu_ctx->pci.config_space == NULL) {
         vfu_ctx->pci.config_space = calloc(1, cfg_reg->size);
         if (vfu_ctx->pci.config_space == NULL) {
-            return ERROR(ENOMEM);
+            return ERROR_INT(ENOMEM);
         }
     }
 
@@ -1015,7 +1015,7 @@ vfu_realize_ctx(vfu_ctx_t *vfu_ctx)
         vfu_ctx->irqs = calloc(1, sizeof(vfu_irqs_t) + size);
         if (vfu_ctx->irqs == NULL) {
             // vfu_ctx->pci.config_space should be free'ed by vfu_destroy_ctx().
-            return ERROR(ENOMEM);
+            return ERROR_INT(ENOMEM);
         }
 
         // Set context irq information.
@@ -1051,7 +1051,7 @@ vfu_run_ctx(vfu_ctx_t *vfu_ctx)
     assert(vfu_ctx != NULL);
 
     if (!vfu_ctx->realized) {
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     blocking = !(vfu_ctx->flags & LIBVFIO_USER_FLAG_ATTACH_NB);
@@ -1131,19 +1131,16 @@ vfu_create_ctx(vfu_trans_t trans, const char *path, int flags, void *pvt,
     //FIXME: Validate arguments.
 
     if (trans != VFU_TRANS_SOCK) {
-        errno = ENOTSUP;
-        return NULL;
+        return ERROR_PTR(ENOTSUP);
     }
 
     if (dev_type != VFU_DEV_TYPE_PCI) {
-        errno = EINVAL;
-        return NULL;
+        return ERROR_PTR(EINVAL);
     }
 
     vfu_ctx = calloc(1, sizeof(vfu_ctx_t));
     if (vfu_ctx == NULL) {
-        errno = ENOMEM;
-        return NULL;
+        return ERROR_PTR(ENOMEM);
     }
 
     vfu_ctx->fd = -1;
@@ -1197,9 +1194,8 @@ vfu_create_ctx(vfu_trans_t trans, const char *path, int flags, void *pvt,
 
 err_out:
     vfu_destroy_ctx(vfu_ctx);
-    errno = -err;
 
-    return NULL;
+    return ERROR_PTR(-err);
 }
 
 int
@@ -1207,7 +1203,7 @@ vfu_setup_log(vfu_ctx_t *vfu_ctx, vfu_log_fn_t *log, int log_level)
 {
 
     if (log_level != LOG_ERR && log_level != LOG_INFO && log_level != LOG_DEBUG) {
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     vfu_ctx->log = log;
@@ -1285,13 +1281,13 @@ vfu_setup_region(vfu_ctx_t *vfu_ctx, int region_idx, size_t size,
     if ((mmap_areas == NULL) != (nr_mmap_areas == 0) ||
         (mmap_areas != NULL && fd == -1)) {
         vfu_log(vfu_ctx, LOG_ERR, "invalid mappable region arguments");
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     if (region_idx < VFU_PCI_DEV_BAR0_REGION_IDX ||
         region_idx >= VFU_PCI_DEV_NUM_REGIONS) {
         vfu_log(vfu_ctx, LOG_ERR, "invalid region index %d", region_idx);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     /*
@@ -1299,19 +1295,19 @@ vfu_setup_region(vfu_ctx_t *vfu_ctx, int region_idx, size_t size,
      */
     if (region_idx == VFU_PCI_DEV_CFG_REGION_IDX &&
         flags != VFU_REGION_FLAG_RW) {
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     if (region_idx == VFU_PCI_DEV_MIGR_REGION_IDX &&
         size < vfu_get_migr_register_area_size()) {
         vfu_log(vfu_ctx, LOG_ERR, "invalid migration region size %d", size);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     for (i = 0; i < nr_mmap_areas; i++) {
         struct iovec *iov = &mmap_areas[i];
         if ((size_t)iov->iov_base + iov->iov_len > size) {
-            return ERROR(EINVAL);
+            return ERROR_INT(EINVAL);
         }
     }
 
@@ -1352,7 +1348,7 @@ out:
     if (ret < 0) {
         free(reg->mmap_areas);
         memset(reg, 0, sizeof (*reg));
-        return ERROR(-ret);
+        return ERROR_INT(-ret);
     }
     return 0;
 }
@@ -1377,7 +1373,7 @@ vfu_setup_device_dma_cb(vfu_ctx_t *vfu_ctx, vfu_map_dma_cb_t *map_dma,
     // Create the internal DMA controller.
     vfu_ctx->dma = dma_controller_create(vfu_ctx, VFU_DMA_REGIONS);
     if (vfu_ctx->dma == NULL) {
-        return ERROR(ENOMEM);
+        return ERROR_INT(ENOMEM);
     }
 
     vfu_ctx->map_dma = map_dma;
@@ -1397,7 +1393,7 @@ vfu_setup_device_nr_irqs(vfu_ctx_t *vfu_ctx, enum vfu_dev_irq_type type,
         vfu_log(vfu_ctx, LOG_ERR, "Invalid IRQ index %d, should be between "
                "(%d to %d)", type, VFU_DEV_INTX_IRQ,
                VFU_DEV_REQ_IRQ);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     vfu_ctx->irq_count[type] = count;
@@ -1417,19 +1413,19 @@ vfu_setup_device_migration_callbacks(vfu_ctx_t *vfu_ctx,
 
     if (vfu_ctx->migr_reg == NULL) {
         vfu_log(vfu_ctx, LOG_ERR, "no device migration region");
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     if (callbacks->version != VFU_MIGR_CALLBACKS_VERS) {
         vfu_log(vfu_ctx, LOG_ERR, "unsupported migration callbacks version %d",
                 callbacks->version);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     vfu_ctx->migration = init_migration(callbacks, data_offset, &ret);
     if (vfu_ctx->migration == NULL) {
         vfu_log(vfu_ctx, LOG_ERR, "failed to initialize device migration");
-        return ERROR(ret);
+        return ERROR_INT(ret);
     }
 
     return 0;
@@ -1449,8 +1445,7 @@ vfu_addr_to_sg(vfu_ctx_t *vfu_ctx, dma_addr_t dma_addr,
     assert(vfu_ctx != NULL);
 
     if (unlikely(vfu_ctx->unmap_dma == NULL)) {
-        errno = EINVAL;
-        return -1;
+        return ERROR_INT(EINVAL);
     }
 
     return dma_addr_to_sg(vfu_ctx->dma, dma_addr, len, sg, max_sg, prot);
@@ -1460,11 +1455,18 @@ inline int
 vfu_map_sg(vfu_ctx_t *vfu_ctx, const dma_sg_t *sg,
 	       struct iovec *iov, int cnt)
 {
+    int ret;
+
     if (unlikely(vfu_ctx->unmap_dma == NULL)) {
-        errno = EINVAL;
-        return -1;
+        return ERROR_INT(EINVAL);
+    }
+
+    ret = dma_map_sg(vfu_ctx->dma, sg, iov, cnt);
+    if (ret < 0) {
+        return ERROR_INT(-ret);
     }
-    return dma_map_sg(vfu_ctx->dma, sg, iov, cnt);
+
+    return 0;
 }
 
 inline void
@@ -1491,7 +1493,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 -ENOMEM;
+        return ERROR_INT(ENOMEM);
     }
 
     dma_send.addr = sg->dma_addr;
@@ -1502,7 +1504,7 @@ vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
     memcpy(data, dma_recv->data, sg->length); /* FIXME no need for memcpy */
     free(dma_recv);
 
-    return ret;
+    return ret < 0 ? ERROR_INT(-ret) : 0;
 }
 
 int
@@ -1517,7 +1519,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 -ENOMEM;
+        return ERROR_INT(ENOMEM);
     }
     dma_send->addr = sg->dma_addr;
     dma_send->count = sg->length;
@@ -1527,7 +1529,7 @@ vfu_dma_write(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
                                    &dma_recv, sizeof(dma_recv));
     free(dma_send);
 
-    return ret;
+    return ret < 0 ? ERROR_INT(-ret) : 0;
 }
 
 uint64_t
diff --git a/lib/pci.c b/lib/pci.c
index 78ae459..8d40acb 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -418,12 +418,12 @@ vfu_pci_init(vfu_ctx_t *vfu_ctx, vfu_pci_type_t pci_type,
         break;
     default:
         vfu_log(vfu_ctx, LOG_ERR, "invalid PCI type %u", pci_type);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
         vfu_log(vfu_ctx, LOG_ERR, "invalid PCI header type %d", hdr_type);
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     /*
@@ -433,13 +433,13 @@ vfu_pci_init(vfu_ctx_t *vfu_ctx, vfu_pci_type_t pci_type,
     if (vfu_ctx->pci.config_space != NULL) {
         vfu_log(vfu_ctx, LOG_ERR,
                 "PCI configuration space header already setup");
-        return ERROR(EEXIST);
+        return ERROR_INT(EEXIST);
     }
 
     // Allocate a buffer for the config space.
     cfg_space = calloc(1, size);
     if (cfg_space == NULL) {
-        return ERROR(ENOMEM);
+        return ERROR_INT(ENOMEM);
     }
 
     vfu_ctx->pci.type = pci_type;
diff --git a/lib/pci_caps.c b/lib/pci_caps.c
index 26a8ba9..8adf2f7 100644
--- a/lib/pci_caps.c
+++ b/lib/pci_caps.c
@@ -535,12 +535,12 @@ vfu_pci_add_capability(vfu_ctx_t *vfu_ctx, size_t pos, int flags, void *data)
 
     if (flags & ~(VFU_CAP_FLAG_EXTENDED | VFU_CAP_FLAG_CALLBACK |
         VFU_CAP_FLAG_READONLY)) {
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     if ((flags & VFU_CAP_FLAG_CALLBACK) &&
         vfu_ctx->reg_info[VFU_PCI_DEV_CFG_REGION_IDX].cb == NULL) {
-        return ERROR(EINVAL);
+        return ERROR_INT(EINVAL);
     }
 
     cap.off = pos;
@@ -553,11 +553,11 @@ vfu_pci_add_capability(vfu_ctx_t *vfu_ctx, size_t pos, int flags, void *data)
         case VFU_PCI_TYPE_EXPRESS:
             break;
         default:
-            return ERROR(EINVAL);
+            return ERROR_INT(EINVAL);
         }
 
         if (vfu_ctx->pci.nr_ext_caps == VFU_MAX_CAPS) {
-            return ERROR(ENOSPC);
+            return ERROR_INT(ENOSPC);
         }
 
         cap.id = ((struct pcie_ext_cap_hdr *)data)->id;
@@ -575,20 +575,20 @@ vfu_pci_add_capability(vfu_ctx_t *vfu_ctx, size_t pos, int flags, void *data)
             break;
         default:
             vfu_log(vfu_ctx, LOG_ERR, "unsupported capability %#x\n", cap.id);
-            return ERROR(ENOTSUP);
+            return ERROR_INT(ENOTSUP);
         }
 
         cap.size = cap_size(vfu_ctx, data, extended);
 
         if (cap.off + cap.size >= pci_config_space_size(vfu_ctx)) {
-            return ERROR(EINVAL);
+            return ERROR_INT(EINVAL);
         }
 
         ret = ext_cap_place(vfu_ctx, &cap, data);
 
     } else {
         if (vfu_ctx->pci.nr_caps == VFU_MAX_CAPS) {
-            return ERROR(ENOSPC);
+            return ERROR_INT(ENOSPC);
         }
 
         cap.id = ((struct cap_hdr *)data)->id;
@@ -614,20 +614,20 @@ vfu_pci_add_capability(vfu_ctx_t *vfu_ctx, size_t pos, int flags, void *data)
             break;
         default:
             vfu_log(vfu_ctx, LOG_ERR, "unsupported capability %#x\n", cap.id);
-            return ERROR(ENOTSUP);
+            return ERROR_INT(ENOTSUP);
         }
 
         cap.size = cap_size(vfu_ctx, data, extended);
 
         if (cap.off + cap.size >= pci_config_space_size(vfu_ctx)) {
-            return ERROR(EINVAL);
+            return ERROR_INT(EINVAL);
         }
 
         ret = cap_place(vfu_ctx, &cap, data);
     }
 
     if (ret != 0) {
-        return ERROR(ret);
+        return ERROR_INT(ret);
     }
 
     if (extended) {
diff --git a/lib/private.h b/lib/private.h
index 98b94ff..d1176ff 100644
--- a/lib/private.h
+++ b/lib/private.h
@@ -37,12 +37,19 @@
 #include "dma.h"
 
 static inline int
-ERROR(int err)
+ERROR_INT(int err)
 {
     errno = err;
     return -1;
 }
 
+static inline void *
+ERROR_PTR(int err)
+{
+    errno = err;
+    return NULL;
+}
+
 struct transport_ops {
     int (*init)(vfu_ctx_t *vfu_ctx);
     int (*attach)(vfu_ctx_t *vfu_ctx);