libxl: Have flexarray using the GC

This patch makes the flexarray function libxl__gc aware.

It also updates every function that use a flexarray to pass the gc and removes
every memory allocation check and free.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/flexarray.c b/tools/libxl/flexarray.c
index edf616ca9a00f67b4bc95f7774e58ad257ee7b06..fe40e762e45d2d2f5431e1651249e3f746c9d191 100644 (file)
--- a/tools/libxl/flexarray.c
+++ b/tools/libxl/flexarray.c
@@ -16,36 +16,43 @@
 #include "libxl_internal.h"
 #include <stdarg.h>
 
-flexarray_t *flexarray_make(int size, int autogrow)
+/*
+ * It is safe to store gc in the struct because:
+ * - If it an actual gc, then the flexarray should not be used after the gc
+ *   have been freed.
+ * - If it is a NOGC, then this point to a structure embedded in libxl_ctx,
+ *   therefore will survive across several libxl calls.
+ */
+
+flexarray_t *flexarray_make(libxl__gc *gc, int size, int autogrow)
 {
-    flexarray_t *array = malloc(sizeof(struct flexarray));
-    if (array) {
-        array->size = size;
-        array->autogrow = autogrow;
-        array->count = 0;
-        array->data = calloc(size, sizeof(void *));
-    }
+    flexarray_t *array;
+
+    GCNEW(array);
+    array->size = size;
+    array->autogrow = autogrow;
+    array->count = 0;
+    array->gc = gc;
+    GCNEW_ARRAY(array->data, size);
+
     return array;
 }
 
 void flexarray_free(flexarray_t *array)
 {
+    assert(!libxl__gc_is_real(array->gc));
     free(array->data);
     free(array);
 }
 
-int flexarray_grow(flexarray_t *array, int extents)
+void flexarray_grow(flexarray_t *array, int extents)
 {
-    void **data;
     int newsize;
+    libxl__gc *gc = array->gc;
 
     newsize = array->size + extents;
-    data = realloc(array->data, sizeof(void *) * newsize);
-    if (!data)
-        return 1;
+    GCREALLOC_ARRAY(array->data, newsize);
     array->size += extents;
-    array->data = data;
-    return 0;
 }
 
 int flexarray_set(flexarray_t *array, unsigned int idx, void *ptr)
@@ -55,8 +62,7 @@ int flexarray_set(flexarray_t *array, unsigned int idx, void *ptr)
         if (!array->autogrow)
             return 1;
         newsize = (array->size * 2 < idx) ? idx + 1 : array->size * 2;
-        if (flexarray_grow(array, newsize - array->size))
-            return 2;
+        flexarray_grow(array, newsize - array->size);
     }
     if ( idx + 1 > array->count )
         array->count = idx + 1;
@@ -104,7 +110,8 @@ void **flexarray_contents(flexarray_t *array)
 {
     void **data;
     data = array->data;
-    free(array);
+    if (!libxl__gc_is_real(array->gc))
+        free(array);
     return data;
 }
 

diff --git a/tools/libxl/flexarray.h b/tools/libxl/flexarray.h
index ae17f3b14e24f0b8ac79ca57fc599799586ad3c3..aade4179068acc51e857cc195e07e42505a8d42c 100644 (file)
--- a/tools/libxl/flexarray.h
+++ b/tools/libxl/flexarray.h
@@ -16,16 +16,19 @@
 #ifndef FLEXARRAY_H
 #define FLEXARRAY_H
 
+struct libxl__gc;
+
 typedef struct flexarray {
     int size;
     int autogrow;
     unsigned int count;
     void **data; /* array of pointer */
+    struct libxl__gc *gc;
 } flexarray_t;
 
-_hidden flexarray_t *flexarray_make(int size, int autogrow);
+_hidden flexarray_t *flexarray_make(struct libxl__gc *gc, int size, int autogrow);
 _hidden void flexarray_free(flexarray_t *array);
-_hidden int flexarray_grow(flexarray_t *array, int extents);
+_hidden void flexarray_grow(flexarray_t *array, int extents);
 _hidden int flexarray_set(flexarray_t *array, unsigned int index, void *ptr);
 _hidden int flexarray_append(flexarray_t *array, void *ptr);
 _hidden int flexarray_append_pair(flexarray_t *array, void *ptr1, void *ptr2);

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1606eb16a14f40c564b3c203ea9dbcc17789c9bf..af3682f76df1aff0fc06f93021e9555033be5450 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1820,27 +1820,15 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         rc = libxl__device_disk_setdefault(gc, disk);
         if (rc) goto out;
 
-        if (front)
-            flexarray_free(front);
-        front = flexarray_make(16, 1);
-        if (!front) {
-            rc = ERROR_NOMEM;
-            goto out;
-        }
-        if (back)
-            flexarray_free(back);
-        back = flexarray_make(16, 1);
-        if (!back) {
-            rc = ERROR_NOMEM;
-            goto out_free;
-        }
+        front = flexarray_make(gc, 16, 1);
+        back = flexarray_make(gc, 16, 1);
 
         GCNEW(device);
         rc = libxl__device_from_disk(gc, domid, disk, device);
         if (rc != 0) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
                    " virtual disk identifier %s", disk->vdev);
-            goto out_free;
+            goto out;
         }
 
         switch (disk->backend) {
@@ -1878,7 +1866,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                     LOG(ERROR, "failed to get blktap devpath for %p\n",
                         disk->pdev_path);
                     rc = ERROR_FAIL;
-                    goto out_free;
+                    goto out;
                 }
                 flexarray_append(back, "tapdisk-params");
                 flexarray_append(back, libxl__sprintf(gc, "%s:%s",
@@ -1900,7 +1888,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
             default:
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
                 rc = ERROR_INVAL;
-                goto out_free;
+                goto out;
         }
 
         flexarray_append(back, "frontend-id");
@@ -1937,7 +1925,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
-        if (rc < 0) goto out_free;
+        if (rc < 0) goto out;
     }
 
     aodev->dev = device;
@@ -1946,9 +1934,6 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
 
     rc = 0;
 
-out_free:
-    flexarray_free(back);
-    flexarray_free(front);
 out:
     libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
@@ -2204,7 +2189,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
     if (rc) goto out;
     path = libxl__device_backend_path(gc, &device);
 
-    insert = flexarray_make(4, 1);
+    insert = flexarray_make(gc, 4, 1);
 
     flexarray_append_pair(insert, "type",
                           libxl__device_disk_string_of_backend(disk->backend));
@@ -2230,8 +2215,6 @@ out:
         libxl_device_disk_dispose(&disks[i]);
     free(disks);
 
-    if (insert) flexarray_free(insert);
-
     if (rc) return AO_ABORT(rc);
     return AO_INPROGRESS;
 }
@@ -2567,21 +2550,13 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     rc = libxl__device_nic_setdefault(gc, nic, domid);
     if (rc) goto out;
 
-    front = flexarray_make(16, 1);
-    if (!front) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-    back = flexarray_make(16, 1);
-    if (!back) {
-        rc = ERROR_NOMEM;
-        goto out_free;
-    }
+    front = flexarray_make(gc, 16, 1);
+    back = flexarray_make(gc, 16, 1);
 
     if (nic->devid == -1) {
         if (!(dompath = libxl__xs_get_dompath(gc, domid))) {
             rc = ERROR_FAIL;
-            goto out_free;
+            goto out;
         }
         if (!(l = libxl__xs_directory(gc, XBT_NULL,
                                      libxl__sprintf(gc, "%s/device/vif", dompath), &nb))) {
@@ -2593,7 +2568,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
 
     GCNEW(device);
     rc = libxl__device_from_nic(gc, domid, nic, device);
-    if ( rc != 0 ) goto out_free;
+    if ( rc != 0 ) goto out;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(gc, "%d", domid));
@@ -2652,9 +2627,6 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     libxl__wait_device_connection(egc, aodev);
 
     rc = 0;
-out_free:
-    flexarray_free(back);
-    flexarray_free(front);
 out:
     aodev->rc = rc;
     if (rc) aodev->callback(egc, aodev);
@@ -2851,16 +2823,8 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
-    front = flexarray_make(16, 1);
-    if (!front) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-    back = flexarray_make(16, 1);
-    if (!back) {
-        rc = ERROR_NOMEM;
-        goto out_free;
-    }
+    front = flexarray_make(gc, 16, 1);
+    back = flexarray_make(gc, 16, 1);
 
     device.backend_devid = console->devid;
     device.backend_domid = console->backend_domid;
@@ -2908,9 +2872,6 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
-out_free:
-    flexarray_free(back);
-    flexarray_free(front);
 out:
     return rc;
 }
@@ -2964,19 +2925,11 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
     rc = libxl__device_vkb_setdefault(gc, vkb);
     if (rc) goto out;
 
-    front = flexarray_make(16, 1);
-    if (!front) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-    back = flexarray_make(16, 1);
-    if (!back) {
-        rc = ERROR_NOMEM;
-        goto out_free;
-    }
+    front = flexarray_make(gc, 16, 1);
+    back = flexarray_make(gc, 16, 1);
 
     rc = libxl__device_from_vkb(gc, domid, vkb, &device);
-    if (rc != 0) goto out_free;
+    if (rc != 0) goto out;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(gc, "%d", domid));
@@ -2996,9 +2949,6 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
-out_free:
-    flexarray_free(back);
-    flexarray_free(front);
 out:
     return rc;
 }
@@ -3063,19 +3013,11 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
     rc = libxl__device_vfb_setdefault(gc, vfb);
     if (rc) goto out;
 
-    front = flexarray_make(16, 1);
-    if (!front) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-    back = flexarray_make(16, 1);
-    if (!back) {
-        rc = ERROR_NOMEM;
-        goto out_free;
-    }
+    front = flexarray_make(gc, 16, 1);
+    back = flexarray_make(gc, 16, 1);
 
     rc = libxl__device_from_vfb(gc, domid, vfb, &device);
-    if (rc != 0) goto out_free;
+    if (rc != 0) goto out;
 
     flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d", domid));
     flexarray_append_pair(back, "online", "1");
@@ -3108,9 +3050,6 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
-out_free:
-    flexarray_free(front);
-    flexarray_free(back);
 out:
     return rc;
 }

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 62c995432ad20d209d15d72185715b1742f35684..054da3ed1b62aeced43f8c9061db09785e016077 100644 (file)
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -109,10 +109,7 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
     const char *keymap = dm_keymap(guest_config);
     int i;
     flexarray_t *dm_args;
-    dm_args = flexarray_make(16, 1);
-
-    if (!dm_args)
-        return NULL;
+    dm_args = flexarray_make(gc, 16, 1);
 
     flexarray_vappend(dm_args, dm,
                       "-d", libxl__sprintf(gc, "%d", domid), NULL);
@@ -340,9 +337,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     int i;
     uint64_t ram_size;
 
-    dm_args = flexarray_make(16, 1);
-    if (!dm_args)
-        return NULL;
+    dm_args = flexarray_make(gc, 16, 1);
 
     flexarray_vappend(dm_args, dm,
                       "-xen-domid",
@@ -837,7 +832,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
                          "setting target domain %d -> %d",
                          dm_domid, guest_domid);
         ret = ERROR_FAIL;
-        goto out_free;
+        goto out;
     }
     xs_set_target(ctx->xsh, dm_domid, guest_domid);
 
@@ -861,11 +856,8 @@ retry_transaction:
     libxl__add_disks(egc, ao, dm_domid, dm_config, &sdss->multidev);
     libxl__multidev_prepared(egc, &sdss->multidev, 0);
 
-    free(args);
     return;
 
-out_free:
-    free(args);
 out:
     assert(ret);
     spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
@@ -1165,7 +1157,6 @@ retry_transaction:
 out_close:
     close(null);
     close(logfile_w);
-    free(args);
 out:
     if (rc)
         device_model_spawn_outcome(egc, dmss, rc);

diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 8e17842e41a1abd26923e89bef80add97a5e90d9..2e6322bfa1461390835ec166300e1e5e96f41817 100644 (file)
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -220,13 +220,7 @@ static libxl__json_object *json_object_alloc(libxl__gc *gc,
     obj->type = type;
 
     if (type == JSON_MAP || type == JSON_ARRAY) {
-        flexarray_t *array = flexarray_make(1, 1);
-        if (array == NULL) {
-            LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR,
-                             "Failed to allocate a flexarray");
-            free(obj);
-            return NULL;
-        }
+        flexarray_t *array = flexarray_make(NOGC, 1, 1);
         if (type == JSON_MAP)
             obj->u.map = array;
         else

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index ff447e710260e3486c5e066c0351e9b584991711..eac35c10f204062499511a3b670aac81ee483cb9 100644 (file)
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -73,12 +73,8 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
     libxl__device device;
     int ret = ERROR_NOMEM, i;
 
-    front = flexarray_make(16, 1);
-    if (!front)
-        goto out;
-    back = flexarray_make(16, 1);
-    if (!back)
-        goto out;
+    front = flexarray_make(gc, 16, 1);
+    back = flexarray_make(gc, 16, 1);
 
     ret = 0;
 
@@ -108,11 +104,6 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
                               libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
-out:
-    if (back)
-        flexarray_free(back);
-    if (front)
-        flexarray_free(front);
     return 0;
 }
 
@@ -138,9 +129,7 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
             return ERROR_FAIL;
     }
 
-    back = flexarray_make(16, 1);
-    if (!back)
-        return ERROR_NOMEM;
+    back = flexarray_make(gc, 16, 1);
 
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Adding new pci device to xenstore");
     num = atoi(num_devs);
@@ -157,7 +146,6 @@ retry_transaction:
         if (errno == EAGAIN)
             goto retry_transaction;
 
-    flexarray_free(back);
     return 0;
 }
 

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 0c95e6000bf997e92aa5c6b77bca8dde15fc51f4..32f7ac7381861eb2e96f382f565a0125efd587e7 100644 (file)
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -763,7 +763,7 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
     if (!hostaddr)
         return -1;
 
-    parameters = flexarray_make(6, 1);
+    parameters = flexarray_make(gc, 6, 1);
     flexarray_append_pair(parameters, "driver", "xen-pci-passthrough");
     flexarray_append_pair(parameters, "id",
                           libxl__sprintf(gc, PCI_PT_QDEV_ID,
@@ -777,8 +777,6 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
                                              PCI_FUNC(pcidev->vdevfn)));
     }
     args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
-    if (!args)
-        return -1;
 
     rc = qmp_synchronous_send(qmp, "device_add", &args,
                               NULL, NULL, qmp->timeout);
@@ -787,7 +785,6 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
                                   pci_add_callback, pcidev, qmp->timeout);
     }
 
-    flexarray_free(parameters);
     libxl__qmp_close(qmp);
     return rc;
 }
@@ -803,16 +800,13 @@ static int qmp_device_del(libxl__gc *gc, int domid, char *id)
     if (!qmp)
         return ERROR_FAIL;
 
-    parameters = flexarray_make(2, 1);
+    parameters = flexarray_make(gc, 2, 1);
     flexarray_append_pair(parameters, "id", id);
     args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
-    if (!args)
-        return ERROR_NOMEM;
 
     rc = qmp_synchronous_send(qmp, "device_del", &args,
                               NULL, NULL, qmp->timeout);
 
-    flexarray_free(parameters);
     libxl__qmp_close(qmp);
     return rc;
 }
@@ -838,24 +832,13 @@ int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
     if (!qmp)
         return ERROR_FAIL;
 
-    parameters = flexarray_make(2, 1);
-    if (!parameters) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
+    parameters = flexarray_make(gc, 2, 1);
     flexarray_append_pair(parameters, "filename", (char *)filename);
     args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
-    if (!args) {
-        rc = ERROR_NOMEM;
-        goto out2;
-    }
 
     rc = qmp_synchronous_send(qmp, "xen-save-devices-state", &args,
                               NULL, NULL, qmp->timeout);
 
-out2:
-    flexarray_free(parameters);
-out:
     libxl__qmp_close(qmp);
     return rc;
 }
@@ -867,19 +850,16 @@ static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
     libxl_key_value_list args = NULL;
     int rc = 0;
 
-    parameters = flexarray_make(6, 1);
+    parameters = flexarray_make(gc, 6, 1);
     flexarray_append_pair(parameters, "device", device);
     flexarray_append_pair(parameters, "target", target);
     if (arg)
         flexarray_append_pair(parameters, "arg", arg);
     args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
-    if (!args)
-        return ERROR_NOMEM;
 
     rc = qmp_synchronous_send(qmp, "change", &args,
                               NULL, NULL, qmp->timeout);
 
-    flexarray_free(parameters);
     return rc;
 }