[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 4/4] command: ease use with virBuffer



* src/util/command.h (virCommandAddArgBuffer)
(virCommandAddEnvBuffer): New prototypes.
* src/util/command.c (virCommandAddArgBuffer)
(virCommandAddEnvBuffer): Implement them.
* src/libvirt_private.syms (command.h): Export them.
* src/qemu/qemu_conf.c (qemudBuildCommandLine): Use them, plugging
a memory leak on rbd_hosts in the process.
---

Transferring malloc'd contents from one struct to another can be
tricky; hopefully this is easy enough to understand and hard enough to
abuse to warrant the syntactic shorthand that it provides to
qemu_conf.  It helps that this is a transfer from one robust wrapper
to another; since we do NOT want something more direct, like:

/* Add malloc'd str to cmd, transferring ownership of who should free it */
void virCommandTransferArg(virCommandPtr cmd, char *str);

because that just invites bugs from freeing in the wrong place.

 src/libvirt_private.syms |    2 ++
 src/qemu/qemu_conf.c     |   46 +++++++++-------------------------------------
 src/util/command.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
 src/util/command.h       |   17 +++++++++++++++++
 4 files changed, 71 insertions(+), 37 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5a114c..b24ca70 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -85,10 +85,12 @@ virCgroupSetSwapHardLimit;

 # command.h
 virCommandAddArg;
+virCommandAddArgBuffer;
 virCommandAddArgFormat;
 virCommandAddArgList;
 virCommandAddArgPair;
 virCommandAddArgSet;
+virCommandAddEnvBuffer;
 virCommandAddEnvPair;
 virCommandAddEnvPass;
 virCommandAddEnvPassCommon;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4f19037..f4b524e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -4466,7 +4466,6 @@ qemudBuildCommandLine(virConnectPtr conn,
         }
         if (def->os.nBootDevs) {
             virBuffer boot_buf = VIR_BUFFER_INITIALIZER;
-            char *bootstr;
             virCommandAddArg(cmd, "-boot");

             boot[def->os.nBootDevs] = '\0';
@@ -4481,14 +4480,7 @@ qemudBuildCommandLine(virConnectPtr conn,
                 virBufferVSprintf(&boot_buf, "%s", boot);
             }

-            if (virBufferError(&boot_buf)) {
-                virReportOOMError();
-                goto error;
-            }
-
-            bootstr = virBufferContentAndReset(&boot_buf);
-            virCommandAddArg(cmd, bootstr);
-            VIR_FREE(bootstr);
+            virCommandAddArgBuffer(cmd, &boot_buf);
         }

         if (def->os.kernel)
@@ -4622,7 +4614,7 @@ qemudBuildCommandLine(virConnectPtr conn,
                 disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
                 for (j = 0 ; j < disk->nhosts ; j++) {
                     if (!has_rbd_hosts) {
-                        virBufferAddLit(&rbd_hosts, "-m ");
+                        virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
                         has_rbd_hosts = true;
                     } else {
                         virBufferAddLit(&rbd_hosts, ",");
@@ -4727,7 +4719,7 @@ qemudBuildCommandLine(virConnectPtr conn,
                     snprintf(file, PATH_MAX, "rbd:%s,", disk->src);
                     for (j = 0 ; j < disk->nhosts ; j++) {
                         if (!has_rbd_hosts) {
-                            virBufferAddLit(&rbd_hosts, "-m ");
+                            virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
                             has_rbd_hosts = true;
                         } else {
                             virBufferAddLit(&rbd_hosts, ",");
@@ -4761,15 +4753,8 @@ qemudBuildCommandLine(virConnectPtr conn,
         }
     }

-    if (virBufferError(&rbd_hosts)) {
-        virBufferFreeAndReset(&rbd_hosts);
-        goto no_memory;
-    }
-    if (has_rbd_hosts) {
-        char *optstr = virBufferContentAndReset(&rbd_hosts);
-        virCommandAddEnvPair(cmd, "CEPH_ARGS", optstr);
-        VIR_FREE(optstr);
-    }
+    if (has_rbd_hosts)
+        virCommandAddEnvBuffer(cmd, &rbd_hosts);

     if (qemuCmdFlags & QEMUD_CMD_FLAG_FSDEV) {
         for (i = 0 ; i < def->nfss ; i++) {
@@ -5082,7 +5067,6 @@ qemudBuildCommandLine(virConnectPtr conn,
     if ((def->ngraphics == 1) &&
         def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
         virBuffer opt = VIR_BUFFER_INITIALIZER;
-        char *optstr;

         if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) {
             if (def->graphics[0]->data.vnc.listenAddr)
@@ -5121,15 +5105,9 @@ qemudBuildCommandLine(virConnectPtr conn,
             virBufferVSprintf(&opt, "%d",
                               def->graphics[0]->data.vnc.port - 5900);
         }
-        if (virBufferError(&opt)) {
-            virBufferFreeAndReset(&opt);
-            goto no_memory;
-        }
-
-        optstr = virBufferContentAndReset(&opt);

-        virCommandAddArgList(cmd, "-vnc", optstr, NULL);
-        VIR_FREE(optstr);
+        virCommandAddArg(cmd, "-vnc");
+        virCommandAddArgBuffer(cmd, &opt);
         if (def->graphics[0]->data.vnc.keymap) {
             virCommandAddArgList(cmd, "-k", def->graphics[0]->data.vnc.keymap,
                                  NULL);
@@ -5171,7 +5149,6 @@ qemudBuildCommandLine(virConnectPtr conn,
     } else if ((def->ngraphics == 1) &&
                def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
         virBuffer opt = VIR_BUFFER_INITIALIZER;
-        char *optstr;

         if (!(qemuCmdFlags & QEMUD_CMD_FLAG_SPICE)) {
             qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -5214,13 +5191,8 @@ qemudBuildCommandLine(virConnectPtr conn,
             }
         }

-        if (virBufferError(&opt))
-            goto no_memory;
-
-        optstr = virBufferContentAndReset(&opt);
-
-        virCommandAddArgList(cmd, "-spice", optstr, NULL);
-        VIR_FREE(optstr);
+        virCommandAddArg(cmd, "-spice");
+        virCommandAddArgBuffer(cmd, &opt);
         if (def->graphics[0]->data.spice.keymap)
             virCommandAddArgList(cmd, "-k",
                                  def->graphics[0]->data.spice.keymap, NULL);
diff --git a/src/util/command.c b/src/util/command.c
index 5e2b19a..90c0d3a 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -315,6 +315,28 @@ virCommandAddEnvString(virCommandPtr cmd, const char *str)


 /*
+ * Convert a buffer containing preformatted name=value into an
+ * environment variable of the child
+ */
+void
+virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
+{
+    if (!cmd || cmd->has_error)
+        return;
+
+    /* env plus trailing NULL. */
+    if (virBufferError(buf) ||
+        VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 1 + 1) < 0) {
+        cmd->has_error = ENOMEM;
+        virBufferFreeAndReset(buf);
+        return;
+    }
+
+    cmd->env[cmd->nenv++] = virBufferContentAndReset(buf);
+}
+
+
+/*
  * Pass an environment variable to the child
  * using current process' value
  */
@@ -381,6 +403,27 @@ virCommandAddArg(virCommandPtr cmd, const char *val)


 /*
+ * Convert a buffer into a command line argument to the child
+ */
+void
+virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf)
+{
+    if (!cmd || cmd->has_error)
+        return;
+
+    /* Arg plus trailing NULL. */
+    if (virBufferError(buf) ||
+        VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
+        cmd->has_error = ENOMEM;
+        virBufferFreeAndReset(buf);
+        return;
+    }
+
+    cmd->args[cmd->nargs++] = virBufferContentAndReset(buf);
+}
+
+
+/*
  * Add a command line argument created by a printf-style format
  */
 void
diff --git a/src/util/command.h b/src/util/command.h
index 9b04e68..59d0ee3 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -24,6 +24,7 @@

 # include "internal.h"
 # include "util.h"
+# include "buf.h"

 typedef struct _virCommand virCommand;
 typedef virCommand *virCommandPtr;
@@ -110,6 +111,15 @@ void virCommandAddEnvPair(virCommandPtr cmd,
  */
 void virCommandAddEnvString(virCommandPtr cmd,
                             const char *str) ATTRIBUTE_NONNULL(2);
+
+/*
+ * Convert a buffer containing preformatted name=value into an
+ * environment variable of the child.
+ * Correctly transfers memory errors or contents from buf to cmd.
+ */
+void virCommandAddEnvBuffer(virCommandPtr cmd,
+                            virBufferPtr buf);
+
 /*
  * Pass an environment variable to the child
  * using current process' value
@@ -129,6 +139,13 @@ void virCommandAddArg(virCommandPtr cmd,
                       const char *val) ATTRIBUTE_NONNULL(2);

 /*
+ * Convert a buffer into a command line argument to the child.
+ * Correctly transfers memory errors or contents from buf to cmd.
+ */
+void virCommandAddArgBuffer(virCommandPtr cmd,
+                            virBufferPtr buf);
+
+/*
  * Add a command line argument created by a printf-style format
  */
 void virCommandAddArgFormat(virCommandPtr cmd,
-- 
1.7.3.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]