[v3,for-4.7,11/16] libxl: convert libxl__device_model_xs_path to a macro

Commit Message

Roger Pau Monné April 27, 2016, 11:11 a.m. UTC

Since it's unsafe to code it as a function because it would end up passing a
non literal string to a printf like function.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
Cc: Wei Liu <[e-mail]>
Cc: Ian Jackson <[e-mail]>
---
Changes since v2:
 - Split the libxl__device_model_xs_path convert to a separate patch.
 - s/libxl__device_model_xs_path/DEVICE_MODEL_XS_PATH/
---
 tools/libxl/libxl_device.c      |  2 +-
 tools/libxl/libxl_dm.c          | 20 ++++++++------------
 tools/libxl/libxl_dom.c         |  2 +-
 tools/libxl/libxl_dom_save.c    | 12 ++++++------
 tools/libxl/libxl_dom_suspend.c |  2 +-
 tools/libxl/libxl_internal.c    | 16 ----------------
 tools/libxl/libxl_internal.h    |  7 ++++---
 tools/libxl/libxl_pci.c         | 14 +++++++-------
 8 files changed, 28 insertions(+), 47 deletions(-)

Comments

Wei Liu April 27, 2016, 11:26 a.m. UTC | #1

On Wed, Apr 27, 2016 at 01:11:49PM +0200, Roger Pau Monne wrote:
> Since it's unsafe to code it as a function because it would end up passing a
> non literal string to a printf like function.
> 
> Signed-off-by: Roger Pau Monné <[e-mail]>

Acked-by: Wei Liu <[e-mail]>

Patch

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index a98707c..f79e536 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1264,7 +1264,7 @@  int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
     char *path;
     uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
 
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
     return libxl__xenstore_child_wait_deprecated(gc, domid,
                                      LIBXL_DEVICE_MODEL_START_TIMEOUT,
                                      "Device Model", path, state, spawning,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index fd12844..a1f7bd8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1762,11 +1762,9 @@  void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     perm[1].perms = XS_PERM_READ;
 retry_transaction:
     t = xs_transaction_start(ctx->xsh);
-    xs_mkdir(ctx->xsh, t,
-             libxl__device_model_xs_path(gc, dm_domid, guest_domid, ""));
+    xs_mkdir(ctx->xsh, t, DEVICE_MODEL_XS_PATH(gc, dm_domid, guest_domid, ""));
     xs_set_permissions(ctx->xsh, t,
-                       libxl__device_model_xs_path(gc, dm_domid,
-                                                   guest_domid, ""),
+                       DEVICE_MODEL_XS_PATH(gc, dm_domid, guest_domid, ""),
                        perm, ARRAY_SIZE(perm));
     if (!xs_transaction_end(ctx->xsh, t, 0))
         if (errno == EAGAIN)
@@ -1926,9 +1924,8 @@  static void stubdom_pvqemu_cb(libxl__egc *egc,
     sdss->xswait.ao = ao;
     sdss->xswait.what = GCSPRINTF("Stubdom %u for %u startup",
                                   dm_domid, sdss->dm.guest_domid);
-    sdss->xswait.path =
-        libxl__device_model_xs_path(gc, dm_domid, sdss->dm.guest_domid,
-                                    "/state");
+    sdss->xswait.path = DEVICE_MODEL_XS_PATH(gc, dm_domid, sdss->dm.guest_domid,
+                                             "/state");
     sdss->xswait.timeout_ms = LIBXL_STUBDOM_START_TIMEOUT * 1000;
     sdss->xswait.callback = stubdom_xswait_cb;
     rc = libxl__xswait_start(gc, &sdss->xswait);
@@ -2035,7 +2032,7 @@  void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
         free(path);
     }
 
-    path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID, domid, "");
+    path = DEVICE_MODEL_XS_PATH(gc, LIBXL_TOOLSTACK_DOMID, domid, "");
     xs_mkdir(ctx->xsh, XBT_NULL, path);
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
@@ -2089,8 +2086,8 @@  retry_transaction:
     }
 
     spawn->what = GCSPRINTF("domain %d device model", domid);
-    spawn->xspath = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID,
-                                                domid, "/state");
+    spawn->xspath = DEVICE_MODEL_XS_PATH(gc, LIBXL_TOOLSTACK_DOMID, domid,
+                                         "/state");
     spawn->timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
     spawn->pidpath = GCSPRINTF("%s/image/device-model-pid", dom_path);
     spawn->midproc_cb = libxl__spawn_record_pid;
@@ -2320,8 +2317,7 @@  out:
 
 int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
 {
-    char *path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID,
-                                             domid, "");
+    char *path = DEVICE_MODEL_XS_PATH(gc, LIBXL_TOOLSTACK_DOMID, domid, "");
     if (!xs_rm(CTX->xsh, XBT_NULL, path))
         LOG(ERROR, "xs_rm failed for %s", path);
     /* We should try to destroy the device model anyway. */
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 09d3bca..9b20cf5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1106,7 +1106,7 @@  int libxl__qemu_traditional_cmd(libxl__gc *gc, uint32_t domid,
 {
     char *path = NULL;
     uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/command");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/command");
     return libxl__xs_printf(gc, XBT_NULL, path, "%s", cmd);
 }
 
diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index 821f862..076be04 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -62,10 +62,10 @@  static void domain_suspend_switch_qemu_xen_traditional_logdirty
 
     if (!lds->cmd_path) {
         uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
-        lds->cmd_path = libxl__device_model_xs_path(gc, dm_domid, domid,
-                                                    "/logdirty/cmd");
-        lds->ret_path = libxl__device_model_xs_path(gc, dm_domid, domid,
-                                                    "/logdirty/ret");
+        lds->cmd_path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid,
+                                             "/logdirty/cmd");
+        lds->ret_path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid,
+                                             "/logdirty/ret");
     }
     lds->cmd = enable ? "enable" : "disable";
 
@@ -290,7 +290,7 @@  int libxl__save_emulator_xenstore_data(libxl__domain_save_state *dss,
     const uint32_t domid = dss->domid;
     const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
 
-    xs_root = libxl__device_model_xs_path(gc, dm_domid, domid, "");
+    xs_root = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "");
 
     entries = libxl__xs_directory(gc, 0, GCSPRINTF("%s/physmap", xs_root),
                                   &nr_entries);
@@ -482,7 +482,7 @@  int libxl__restore_emulator_xenstore_data(libxl__domain_create_state *dcs,
 
     const uint32_t domid = dcs->guest_domid;
     const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
-    const char *xs_root = libxl__device_model_xs_path(gc, dm_domid, domid, "");
+    const char *xs_root = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "");
 
     while (next < end) {
         key = next;
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index cc0b217..0648919 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -432,7 +432,7 @@  int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
         uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
 
-        path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+        path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
         state = libxl__xs_read(gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
             libxl__qemu_traditional_cmd(gc, domid, "continue");
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index e7b765b..3b30f8a 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -554,22 +554,6 @@  void libxl__update_domain_configuration(libxl__gc *gc,
     dst->b_info.video_memkb = src->b_info.video_memkb;
 }
 
-char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
-                                  uint32_t domid, const char *format,  ...)
-{
-    char *s, *fmt;
-    va_list ap;
-
-    fmt = GCSPRINTF("/local/domain/%u/device-model/%u%s", dm_domid,
-                    domid, format);
-
-    va_start(ap, format);
-    s = libxl__vsprintf(gc, fmt, ap);
-    va_end(ap);
-
-    return s;
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 662d01a..c791418 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1995,9 +1995,10 @@  _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
 _hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
   /* Return the system-wide default device model */
 _hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
-_hidden char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
-                                          uint32_t domid,
-                                          const char *format, ...) PRINTF_ATTRIBUTE(4, 5);
+
+#define DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, fmt, _a...)              \
+    libxl__sprintf(gc, "/local/domain/%u/device-model/%u" fmt, dm_domid,   \
+                   domid, ##_a)
 
 /*
  * Calling context and GC for event-generating functions:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 300fd4d..ce8d763 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -970,9 +970,9 @@  static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
     uint32_t dm_domid;
 
     dm_domid = libxl_get_stubdom_id(CTX, domid);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
     state = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/parameter");
     if (pcidev->vdevfn) {
         libxl__xs_printf(gc, XBT_NULL, path, PCI_BDF_VDEVFN","PCI_OPTIONS,
                          pcidev->domain, pcidev->bus, pcidev->dev,
@@ -987,9 +987,9 @@  static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
     libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
     rc = libxl__wait_for_device_model_deprecated(gc, domid, NULL, NULL,
                                       pci_ins_check, state);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/parameter");
     vdevfn = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
     if ( rc < 0 )
         LOG(ERROR, "qemu refused to add device: %s", vdevfn);
     else if ( sscanf(vdevfn, "0x%x", &pcidev->vdevfn) != 1 ) {
@@ -1310,9 +1310,9 @@  static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
 
     dm_domid = libxl_get_stubdom_id(CTX, domid);
 
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
     state = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/parameter");
     libxl__xs_printf(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
                      pcidev->bus, pcidev->dev, pcidev->func);
 
@@ -1330,7 +1330,7 @@  static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
             return ERROR_FAIL;
         }
     }
-    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
+    path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
     xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
 
     return 0;