Gianni Tedesco
2010-Aug-13 15:02 UTC
[Xen-devel] [PATCH, v2] xl: make libxl_uuid2string internal to libxenlight
libxenlight exports a function libxl_uuid2string which is used
internally in several places but has one external caller in xl.
This means that libxl internal callers leak since they were not
expecting to have to free() the UUID since the per-api-call-gc-lifetime
patch.
Convert libxl_uuid2string to be an internal function which participates
in the callers garbage collection. Eliminate string_of_uuid() macro in
favour of "format" and "arguments" macros suitable for
printf()-like
functions which are made part of the libxl API and fix-up xl callers to
use that to avoid code duplication and enhance readability.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r dc335ebde3b5 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/libxl.c Fri Aug 13 16:03:09 2010 +0100
@@ -90,7 +90,7 @@ int libxl_domain_make(libxl_ctx *ctx, li
xs_transaction_t t;
xen_domain_handle_t handle;
- uuid_string = libxl_uuid2string(ctx, info->uuid);
+ uuid_string = libxl_uuid2string(&gc, info->uuid);
if (!uuid_string) {
libxl_free_all(&gc);
return ERROR_NOMEM;
@@ -453,7 +453,7 @@ int libxl_domain_preserve(libxl_ctx *ctx
return ERROR_NOMEM;
}
- uuid_string = libxl_uuid2string(ctx, new_uuid);
+ uuid_string = libxl_uuid2string(&gc, new_uuid);
if (!uuid_string) {
libxl_free_all(&gc);
return ERROR_NOMEM;
@@ -2785,7 +2785,7 @@ int libxl_set_memory_target(libxl_ctx *c
if (rc != 1 || info.domain != domid)
goto out;
xcinfo2xlinfo(&info, &ptr);
- uuid = libxl_uuid2string(ctx, ptr.uuid);
+ uuid = libxl_uuid2string(&gc, ptr.uuid);
libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc,
"/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024);
if (enforce || !domid)
diff -r dc335ebde3b5 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/libxl.h Fri Aug 13 16:03:09 2010 +0100
@@ -23,6 +23,12 @@
#include <sys/wait.h> /* for pid_t */
typedef uint8_t libxl_uuid[16];
+#define LIBXL_UUID_FMT
"%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
+#define LIBXL_UUID_BYTES(uuid) uuid[0], uuid[1], uuid[2], uuid[3], \
+ uuid[4], uuid[5], uuid[6], uuid[7], \
+ uuid[8], uuid[9], uuid[10], uuid[11], \
+ uuid[12], uuid[13], uuid[14], uuid[15] \
+
typedef uint8_t libxl_mac[6];
@@ -362,7 +368,6 @@ int libxl_run_bootloader(libxl_ctx *ctx,
libxl_device_disk *disk,
uint32_t domid);
-char *libxl_uuid2string(libxl_ctx *ctx, const libxl_uuid uuid);
/* 0 means ERROR_ENOMEM, which we have logged */
/* events handling */
diff -r dc335ebde3b5 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/libxl_dom.c Fri Aug 13 16:03:09 2010 +0100
@@ -442,19 +442,12 @@ int save_device_model(libxl_ctx *ctx, ui
return 0;
}
-char *libxl_uuid2string(libxl_ctx *ctx, const libxl_uuid uuid)
+char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid)
{
- libxl_gc gc = LIBXL_INIT_GC(ctx);
- char *s = string_of_uuid(&gc, uuid);
- char *ret;
- if (!s) {
- XL_LOG(ctx, XL_LOG_ERROR, "cannot allocate for uuid");
- ret = NULL;
- }else{
- ret = strdup(s);
- }
- libxl_free_all(&gc);
- return ret;
+ char *s = libxl_sprintf(gc, LIBXL_UUID_FMT, LIBXL_UUID_BYTES(uuid));
+ if (!s)
+ XL_LOG(libxl_gc_owner(gc), XL_LOG_ERROR, "cannot allocate for
uuid");
+ return s;
}
static const char *userdata_path(libxl_gc *gc, uint32_t domid,
@@ -472,7 +465,7 @@ static const char *userdata_path(libxl_g
" for domain %"PRIu32, domid);
return NULL;
}
- uuid_string = string_of_uuid(gc, info.uuid);
+ uuid_string = libxl_sprintf(gc, LIBXL_UUID_FMT,
LIBXL_UUID_BYTES(info.uuid));
path = libxl_sprintf(gc, "/var/lib/xen/"
"userdata-%s.%s.%s",
diff -r dc335ebde3b5 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/libxl_internal.h Fri Aug 13 16:03:09 2010 +0100
@@ -106,12 +106,6 @@ typedef struct {
#define PRINTF_ATTRIBUTE(x, y) __attribute__((format(printf, x, y)))
-#define UUID_FMT
"%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
-#define string_of_uuid(ctx, u) \
- libxl_sprintf(ctx, UUID_FMT, \
- (u)[0], (u)[1], (u)[2], (u)[3], (u)[4], (u)[5], (u)[6], (u)[7],
\
- (u)[8], (u)[9], (u)[10], (u)[11], (u)[12], (u)[13], (u)[14],
(u)[15])
-
_hidden int xs_writev(struct xs_handle *xsh, xs_transaction_t t, char *dir,
char *kvs[]);
typedef struct {
@@ -249,4 +243,6 @@ _hidden char *libxl_abs_path(libxl_gc *g
_hidden char *_libxl_domid_to_name(libxl_gc *gc, uint32_t domid);
_hidden char *_libxl_poolid_to_name(libxl_gc *gc, uint32_t poolid);
+_hidden char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid);
+
#endif
diff -r dc335ebde3b5 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Fri Aug 13 16:03:09 2010 +0100
@@ -40,8 +40,6 @@
#include "libxlutil.h"
#include "xl.h"
-#define UUID_FMT
"%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
-
#define CHK_ERRNO( call ) ({ \
int chk_errno = (call); \
if (chk_errno < 0) {
\
@@ -391,11 +389,7 @@ static void printf_info(int domid,
printf("\t(oos %d)\n", c_info->oos);
printf("\t(ssidref %d)\n", c_info->ssidref);
printf("\t(name %s)\n", c_info->name);
- printf("\t(uuid " UUID_FMT ")\n",
- (c_info->uuid)[0], (c_info->uuid)[1], (c_info->uuid)[2],
(c_info->uuid)[3],
- (c_info->uuid)[4], (c_info->uuid)[5], (c_info->uuid)[6],
(c_info->uuid)[7],
- (c_info->uuid)[8], (c_info->uuid)[9], (c_info->uuid)[10],
(c_info->uuid)[11],
- (c_info->uuid)[12], (c_info->uuid)[13], (c_info->uuid)[14],
(c_info->uuid)[15]);
+ printf("\t(uuid " LIBXL_UUID_FMT ")\n",
LIBXL_UUID_BYTES(c_info->uuid));
printf("\t(cpupool %s (%d))\n", c_info->poolname,
c_info->poolid);
if (c_info->xsdata)
printf("\t(xsdata contains data)\n");
@@ -2169,10 +2163,8 @@ void list_domains(int verbose, const lib
info[i].dying ? ''d'' : ''-'',
((float)info[i].cpu_time / 1e9));
free(domname);
- if (verbose) {
- char *uuid = libxl_uuid2string(&ctx, info[i].uuid);
- printf(" %s", uuid);
- }
+ if (verbose)
+ printf(" " LIBXL_UUID_FMT,
LIBXL_UUID_BYTES(info[i].uuid));
putchar(''\n'');
}
}
@@ -2192,11 +2184,7 @@ void list_vm(void)
printf("UUID ID name\n");
for (i = 0; i < nb_vm; i++) {
domname = libxl_domid_to_name(&ctx, info[i].domid);
- printf(UUID_FMT " %d %-30s\n",
- info[i].uuid[0], info[i].uuid[1], info[i].uuid[2], info[i].uuid[3],
- info[i].uuid[4], info[i].uuid[5], info[i].uuid[6], info[i].uuid[7],
- info[i].uuid[8], info[i].uuid[9], info[i].uuid[10],
info[i].uuid[11],
- info[i].uuid[12], info[i].uuid[13], info[i].uuid[14],
info[i].uuid[15],
+ printf(LIBXL_UUID_FMT " %d %-30s\n",
LIBXL_UUID_BYTES(info[i].uuid),
info[i].domid, domname);
free(domname);
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Aug-16 16:34 UTC
[Xen-devel] Re: [PATCH, v2] xl: make libxl_uuid2string internal to libxenlight
On Fri, 13 Aug 2010, Gianni Tedesco (3P) wrote:> libxenlight exports a function libxl_uuid2string which is used > internally in several places but has one external caller in xl. > This means that libxl internal callers leak since they were not > expecting to have to free() the UUID since the per-api-call-gc-lifetime > patch. > > Convert libxl_uuid2string to be an internal function which participates > in the callers garbage collection. Eliminate string_of_uuid() macro in > favour of "format" and "arguments" macros suitable for printf()-like > functions which are made part of the libxl API and fix-up xl callers to > use that to avoid code duplication and enhance readability. > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>applied, thanks _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel