This two patches do a bit of cleanup in the memomy managment in libxl, regarding the use of flexarray. The first one modify flexarray_make to take gc as argument and update every user in libxl to pass gc and to not call flexarray_free anymore. The second one does some cleanup only in libxl_json to make it use the gc. Change since v2: - New patch to expose gc_is_real to libxl. Change since v1: - Change order of the two patch to not end up with leaks. - Add a comment on the first patch to tell why it OK to store the gc in the flexarray struct. - Keep the _free functions (libxl__json_object_free and flexarray_free, as they can be used if gc in NOGC. Anthony PERARD (3): libxl: Move gc_is_real to libxl_internal.h. libxl: Have flexarray using the GC libxl_json: Use libxl alloc function. tools/libxl/flexarray.c | 43 +++++++++++-------- tools/libxl/flexarray.h | 7 +++- tools/libxl/libxl.c | 99 +++++++++----------------------------------- tools/libxl/libxl_dm.c | 15 ++----- tools/libxl/libxl_internal.c | 11 ++--- tools/libxl/libxl_internal.h | 5 +++ tools/libxl/libxl_json.c | 93 +++++++++-------------------------------- tools/libxl/libxl_pci.c | 18 ++------ tools/libxl/libxl_qmp.c | 29 ++----------- 9 files changed, 86 insertions(+), 234 deletions(-) -- Anthony PERARD
Anthony PERARD
2012-Oct-05 09:04 UTC
[PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl_internal.c | 11 +++-------- tools/libxl/libxl_internal.h | 5 +++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index 211c8f5..5a8cd38 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -30,16 +30,11 @@ void libxl__alloc_failed(libxl_ctx *ctx, const char *func, #undef L } -static int gc_is_real(const libxl__gc *gc) -{ - return gc->alloc_maxsize >= 0; -} - void libxl__ptr_add(libxl__gc *gc, void *ptr) { int i; - if (!gc_is_real(gc)) + if (!libxl__gc_is_real(gc)) return; if (!ptr) @@ -71,7 +66,7 @@ void libxl__free_all(libxl__gc *gc) void *ptr; int i; - assert(gc_is_real(gc)); + assert(libxl__gc_is_real(gc)); for (i = 0; i < gc->alloc_maxsize; i++) { ptr = gc->alloc_ptrs[i]; @@ -111,7 +106,7 @@ void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size) if (ptr == NULL) { libxl__ptr_add(gc, new_ptr); - } else if (new_ptr != ptr && gc_is_real(gc)) { + } else if (new_ptr != ptr && libxl__gc_is_real(gc)) { for (i = 0; i < gc->alloc_maxsize; i++) { if (gc->alloc_ptrs[i] == ptr) { gc->alloc_ptrs[i] = new_ptr; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index b6f54ba..c0e879d 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -446,6 +446,11 @@ static inline libxl_ctx *libxl__gc_owner(libxl__gc *gc) return gc->owner; } +static inline int libxl__gc_is_real(const libxl__gc *gc) +{ + return gc->alloc_maxsize >= 0; +} + /* * Memory allocation tracking/helpers * -- Anthony PERARD
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> --- tools/libxl/flexarray.c | 43 ++++++++++++--------- tools/libxl/flexarray.h | 7 +++- tools/libxl/libxl.c | 99 ++++++++++-------------------------------------- tools/libxl/libxl_dm.c | 15 ++------ tools/libxl/libxl_json.c | 8 +--- tools/libxl/libxl_pci.c | 18 ++------- tools/libxl/libxl_qmp.c | 28 ++------------ 7 files changed, 60 insertions(+), 158 deletions(-) diff --git a/tools/libxl/flexarray.c b/tools/libxl/flexarray.c index edf616c..fe40e76 100644 --- 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 ae17f3b..aade417 100644 --- 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 1606eb1..af3682f 100644 --- 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 3be7bad..82d2009 100644 --- 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 8e17842..2e6322b 100644 --- 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 ff447e7..eac35c1 100644 --- 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 2c4d269..6bdf924 100644 --- 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; } -- Anthony PERARD
This patch makes use of the libxl allocation API and the GC and removes the check for allocation failure. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> --- tools/libxl/libxl_json.c | 87 +++++++++++------------------------------------- tools/libxl/libxl_qmp.c | 1 - 2 files changed, 19 insertions(+), 69 deletions(-) diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c index 2e6322b..3cba48f 100644 --- a/tools/libxl/libxl_json.c +++ b/tools/libxl/libxl_json.c @@ -210,17 +210,12 @@ static libxl__json_object *json_object_alloc(libxl__gc *gc, { libxl__json_object *obj; - obj = calloc(1, sizeof (libxl__json_object)); - if (obj == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR, - "Failed to allocate a libxl__json_object"); - return NULL; - } + obj = libxl__zalloc(gc, sizeof(*obj)); obj->type = type; if (type == JSON_MAP || type == JSON_ARRAY) { - flexarray_t *array = flexarray_make(NOGC, 1, 1); + flexarray_t *array = flexarray_make(gc, 1, 1); if (type == JSON_MAP) obj->u.map = array; else @@ -250,11 +245,7 @@ static int json_object_append_to(libxl__gc *gc, break; } case JSON_ARRAY: - if (flexarray_append(dst->u.array, obj) == 2) { - LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR, - "Failed to grow a flexarray"); - return -1; - } + flexarray_append(dst->u.array, obj); break; default: LIBXL__LOG(libxl__gc_owner(gc), LIBXL__LOG_ERROR, @@ -387,11 +378,9 @@ static int json_callback_null(void *opaque) DEBUG_GEN(ctx, null); - if ((obj = json_object_alloc(ctx->gc, JSON_NULL)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_NULL); if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } @@ -405,12 +394,10 @@ static int json_callback_boolean(void *opaque, int boolean) DEBUG_GEN_VALUE(ctx, bool, boolean); - if ((obj = json_object_alloc(ctx->gc, - boolean ? JSON_TRUE : JSON_FALSE)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, + boolean ? JSON_TRUE : JSON_FALSE); if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } @@ -442,8 +429,7 @@ static int json_callback_number(void *opaque, const char *s, libxl_yajl_length l goto error; } - if ((obj = json_object_alloc(ctx->gc, JSON_DOUBLE)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_DOUBLE); obj->u.d = d; } else { long long i = strtoll(s, NULL, 10); @@ -452,23 +438,16 @@ static int json_callback_number(void *opaque, const char *s, libxl_yajl_length l goto error; } - if ((obj = json_object_alloc(ctx->gc, JSON_INTEGER)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_INTEGER); obj->u.i = i; } goto out; error: /* If the conversion fail, we just store the original string. */ - if ((obj = json_object_alloc(ctx->gc, JSON_NUMBER)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_NUMBER); - t = malloc(len + 1); - if (t == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, - "Failed to allocate"); - return 0; - } + t = libxl__zalloc(ctx->gc, len + 1); strncpy(t, s, len); t[len] = 0; @@ -476,7 +455,6 @@ error: out: if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } @@ -490,26 +468,17 @@ static int json_callback_string(void *opaque, const unsigned char *str, char *t = NULL; libxl__json_object *obj = NULL; - t = malloc(len + 1); - if (t == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, - "Failed to allocate"); - return 0; - } + t = libxl__zalloc(ctx->gc, len + 1); DEBUG_GEN_STRING(ctx, str, len); strncpy(t, (const char *) str, len); t[len] = 0; - if ((obj = json_object_alloc(ctx->gc, JSON_STRING)) == NULL) { - free(t); - return 0; - } + obj = json_object_alloc(ctx->gc, JSON_STRING); obj->u.string = t; if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } @@ -522,13 +491,9 @@ static int json_callback_map_key(void *opaque, const unsigned char *str, libxl__yajl_ctx *ctx = opaque; char *t = NULL; libxl__json_object *obj = ctx->current; + libxl__gc *gc = ctx->gc; - t = malloc(len + 1); - if (t == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, - "Failed to allocate"); - return 0; - } + t = libxl__zalloc(gc, len + 1); DEBUG_GEN_STRING(ctx, str, len); @@ -536,21 +501,13 @@ static int json_callback_map_key(void *opaque, const unsigned char *str, t[len] = 0; if (libxl__json_object_is_map(obj)) { - libxl__json_map_node *node = malloc(sizeof (libxl__json_map_node)); - if (node == NULL) { - LIBXL__LOG_ERRNO(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, - "Failed to allocate"); - return 0; - } + libxl__json_map_node *node; + GCNEW(node); node->map_key = t; node->obj = NULL; - if (flexarray_append(obj->u.map, node) == 2) { - LIBXL__LOG_ERRNO(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, - "Failed to grow a flexarray"); - return 0; - } + flexarray_append(obj->u.map, node); } else { LIBXL__LOG(libxl__gc_owner(ctx->gc), LIBXL__LOG_ERROR, "Current json object is not a map"); @@ -567,12 +524,10 @@ static int json_callback_start_map(void *opaque) DEBUG_GEN(ctx, map_open); - if ((obj = json_object_alloc(ctx->gc, JSON_MAP)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_MAP); if (ctx->current) { if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } } @@ -609,12 +564,10 @@ static int json_callback_start_array(void *opaque) DEBUG_GEN(ctx, array_open); - if ((obj = json_object_alloc(ctx->gc, JSON_ARRAY)) == NULL) - return 0; + obj = json_object_alloc(ctx->gc, JSON_ARRAY); if (ctx->current) { if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) { - libxl__json_object_free(ctx->gc, obj); return 0; } } @@ -704,8 +657,6 @@ out: LIBXL__LOG(libxl__gc_owner(gc), LIBXL__LOG_ERROR, "yajl error: %s", str); yajl_free_error(yajl_ctx.hand, str); - - libxl__json_object_free(gc, yajl_ctx.head); yajl_ctx_free(&yajl_ctx); return NULL; } diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index 6bdf924..15550e7 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -484,7 +484,6 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) if (o) { rc = qmp_handle_response(qmp, o); - libxl__json_object_free(gc, o); } else { LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR, "Parse error of : %s\n", s); -- Anthony PERARD
Ian Jackson
2012-Oct-05 10:15 UTC
Re: [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h.
Anthony PERARD writes ("[PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h."):> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> (Strictly speaking there is no need to rename the function if it''s going to be static inline, but it''s fine to do so.) Ian.
Anthony PERARD writes ("[PATCH V3 2/3] libxl: Have flexarray using the GC"):> This patch makes the flexarray function libxl__gc aware.Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>