* [PATCH V3 0/3] Cleanup: flexarray taking gc.
@ 2012-10-05 9:04 Anthony PERARD
2012-10-05 9:04 ` [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h Anthony PERARD
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Anthony PERARD @ 2012-10-05 9:04 UTC (permalink / raw)
To: Xen Devel; +Cc: Anthony PERARD, Ian Jackson, Ian Campbell
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h.
2012-10-05 9:04 [PATCH V3 0/3] Cleanup: flexarray taking gc Anthony PERARD
@ 2012-10-05 9:04 ` Anthony PERARD
2012-10-05 10:15 ` Ian Jackson
2012-10-05 9:04 ` [PATCH V3 2/3] libxl: Have flexarray using the GC Anthony PERARD
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2012-10-05 9:04 UTC (permalink / raw)
To: Xen Devel; +Cc: Anthony PERARD, Ian Jackson, Ian Campbell
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V3 2/3] libxl: Have flexarray using the GC
2012-10-05 9:04 [PATCH V3 0/3] Cleanup: flexarray taking gc Anthony PERARD
2012-10-05 9:04 ` [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h Anthony PERARD
@ 2012-10-05 9:04 ` Anthony PERARD
2012-10-05 10:15 ` Ian Jackson
2012-10-05 9:04 ` [PATCH V3 3/3] libxl_json: Use libxl alloc function Anthony PERARD
2012-10-05 13:44 ` [PATCH V3 0/3] Cleanup: flexarray taking gc Ian Campbell
3 siblings, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2012-10-05 9:04 UTC (permalink / raw)
To: Xen Devel; +Cc: Anthony PERARD, Ian Jackson, Ian Campbell
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V3 3/3] libxl_json: Use libxl alloc function.
2012-10-05 9:04 [PATCH V3 0/3] Cleanup: flexarray taking gc Anthony PERARD
2012-10-05 9:04 ` [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h Anthony PERARD
2012-10-05 9:04 ` [PATCH V3 2/3] libxl: Have flexarray using the GC Anthony PERARD
@ 2012-10-05 9:04 ` Anthony PERARD
2012-10-05 13:44 ` [PATCH V3 0/3] Cleanup: flexarray taking gc Ian Campbell
3 siblings, 0 replies; 7+ messages in thread
From: Anthony PERARD @ 2012-10-05 9:04 UTC (permalink / raw)
To: Xen Devel; +Cc: Anthony PERARD, Ian Jackson, Ian Campbell
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h.
2012-10-05 9:04 ` [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h Anthony PERARD
@ 2012-10-05 10:15 ` Ian Jackson
0 siblings, 0 replies; 7+ messages in thread
From: Ian Jackson @ 2012-10-05 10:15 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Ian Campbell, Xen Devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 2/3] libxl: Have flexarray using the GC
2012-10-05 9:04 ` [PATCH V3 2/3] libxl: Have flexarray using the GC Anthony PERARD
@ 2012-10-05 10:15 ` Ian Jackson
0 siblings, 0 replies; 7+ messages in thread
From: Ian Jackson @ 2012-10-05 10:15 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Ian Campbell, Xen Devel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 0/3] Cleanup: flexarray taking gc.
2012-10-05 9:04 [PATCH V3 0/3] Cleanup: flexarray taking gc Anthony PERARD
` (2 preceding siblings ...)
2012-10-05 9:04 ` [PATCH V3 3/3] libxl_json: Use libxl alloc function Anthony PERARD
@ 2012-10-05 13:44 ` Ian Campbell
3 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2012-10-05 13:44 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Ian Jackson, Xen Devel
On Fri, 2012-10-05 at 10:04 +0100, Anthony PERARD wrote:
> This two patches do a bit of cleanup in the memomy managment in libxl,
> regarding the use of flexarray.
Applied all 3 with IanJ's acks.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-10-05 13:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-05 9:04 [PATCH V3 0/3] Cleanup: flexarray taking gc Anthony PERARD
2012-10-05 9:04 ` [PATCH V3 1/3] libxl: Move gc_is_real to libxl_internal.h Anthony PERARD
2012-10-05 10:15 ` Ian Jackson
2012-10-05 9:04 ` [PATCH V3 2/3] libxl: Have flexarray using the GC Anthony PERARD
2012-10-05 10:15 ` Ian Jackson
2012-10-05 9:04 ` [PATCH V3 3/3] libxl_json: Use libxl alloc function Anthony PERARD
2012-10-05 13:44 ` [PATCH V3 0/3] Cleanup: flexarray taking gc Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.