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>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
committer: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e1ed0519bf2274d595e8a28ef94786a851da8cb2..9638c1676d9fadea2a8a57b96a8338e4388384f5 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -90,7 +90,7 @@ int libxl_domain_make(libxl_ctx *ctx, libxl_domain_create_info *info,
     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, uint32_t domid,
         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;
@@ -2764,7 +2764,7 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memk
     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 --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index fe117377c500be6332e041eb59d789d43ba14bf2..b6e70b690d1144422187b1cb25c0fde61d0fe5c5 100644 (file)
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -132,6 +132,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];
 
@@ -477,7 +483,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 --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 4fe018d7efd739e28a54aae9139dbc35dc0cff41..16918580d0e04715e39f982bd9bda5cc2975eb44 100644 (file)
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -442,19 +442,12 @@ int save_device_model(libxl_ctx *ctx, uint32_t domid, int fd)
     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_gc *gc, uint32_t domid,
                      " 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 --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ec75dbbdb0e48037d8a3476806eb69a95e1bcb8b..ffc213c82f69839054caa441be8af0070e773096 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -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 {
@@ -267,4 +261,6 @@ const char *libxl_blktap_devpath(libxl_gc *gc,
                                  const char *disk,
                                  libxl_disk_phystype phystype);
 
+_hidden char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid);
+
 #endif

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 94a625a39f43a4a9b956ec964c3490a70d642f30..44f1a135b4a5055669254808679f28ddb5beaea3 100644 (file)
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -41,8 +41,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) {                                                \
@@ -393,11 +391,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");
@@ -2188,10 +2182,8 @@ void list_domains(int verbose, const libxl_dominfo *info, int nb_domain)
                 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');
     }
 }
@@ -2211,11 +2203,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);
     }