[libvirt] [PATCHv2 2/3] qemu: simplify list cleanup

Eric Blake eblake at redhat.com
Wed Aug 28 21:01:23 UTC 2013


No need to open code now that we have a nice function.

Interestingly, our virStringFreeList function is typed correctly
(a malloc'd list of malloc'd strings is NOT const, whether at the
point where it is created, or at the point where it is cleand up),
so using it with a 'const char **' argument would require a cast
to keep the compiler.  I chose instead to remove const from code
even where we don't modify the argument, just to avoid the need
to cast.

* src/qemu/qemu_command.h (qemuParseCommandLine): Drop declaration.
* src/qemu/qemu_command.c (qemuParseProcFileStrings)
(qemuStringToArgvEnv): Don't force malloc'd result to be const.
(qemuParseCommandLinePid, qemuParseCommandLineString): Simplify
cleanup.
(qemuParseCommandLine, qemuFindEnv): Drop const-correctness to
avoid the need to cast in callers.

Signed-off-by: Eric Blake <eblake at redhat.com>
---

v2: fix compilation, and extend the cleanup

 src/qemu/qemu_command.c | 72 +++++++++++++++++++------------------------------
 src/qemu/qemu_command.h |  7 -----
 2 files changed, 27 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 270a826..cfdfe7e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9528,8 +9528,8 @@ qemuBuildChrDeviceStr(char **deviceStr,
  * on space
  */
 static int qemuStringToArgvEnv(const char *args,
-                               const char ***retenv,
-                               const char ***retargv)
+                               char ***retenv,
+                               char ***retargv)
 {
     char **arglist = NULL;
     int argcount = 0;
@@ -9538,8 +9538,8 @@ static int qemuStringToArgvEnv(const char *args,
     size_t i;
     const char *curr = args;
     const char *start;
-    const char **progenv = NULL;
-    const char **progargv = NULL;
+    char **progenv = NULL;
+    char **progargv = NULL;

     /* Iterate over string, splitting on sequences of ' ' */
     while (curr && *curr != '\0') {
@@ -9620,12 +9620,8 @@ static int qemuStringToArgvEnv(const char *args,
     return 0;

 error:
-    for (i = 0; progenv && progenv[i]; i++)
-        VIR_FREE(progenv[i]);
-    VIR_FREE(progenv);
-    for (i = 0; i < argcount; i++)
-        VIR_FREE(arglist[i]);
-    VIR_FREE(arglist);
+    virStringFreeList(progenv);
+    virStringFreeList(arglist);
     return -1;
 }

@@ -9633,7 +9629,7 @@ error:
 /*
  * Search for a named env variable, and return the value part
  */
-static const char *qemuFindEnv(const char **progenv,
+static const char *qemuFindEnv(char **progenv,
                                const char *name)
 {
     size_t i;
@@ -10776,13 +10772,14 @@ qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) {
  * virDomainDefPtr representing these settings as closely
  * as is practical. This is not an exact science....
  */
-virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
-                                     virDomainXMLOptionPtr xmlopt,
-                                     const char **progenv,
-                                     const char **progargv,
-                                     char **pidfile,
-                                     virDomainChrSourceDefPtr *monConfig,
-                                     bool *monJSON)
+static virDomainDefPtr
+qemuParseCommandLine(virCapsPtr qemuCaps,
+                     virDomainXMLOptionPtr xmlopt,
+                     char **progenv,
+                     char **progargv,
+                     char **pidfile,
+                     virDomainChrSourceDefPtr *monConfig,
+                     bool *monJSON)
 {
     virDomainDefPtr def;
     size_t i;
@@ -11734,10 +11731,9 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr qemuCaps,
                                            virDomainChrSourceDefPtr *monConfig,
                                            bool *monJSON)
 {
-    const char **progenv = NULL;
-    const char **progargv = NULL;
+    char **progenv = NULL;
+    char **progargv = NULL;
     virDomainDefPtr def = NULL;
-    size_t i;

     if (qemuStringToArgvEnv(args, &progenv, &progargv) < 0)
         goto cleanup;
@@ -11746,13 +11742,8 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr qemuCaps,
                                pidfile, monConfig, monJSON);

 cleanup:
-    for (i = 0; progargv && progargv[i]; i++)
-        VIR_FREE(progargv[i]);
-    VIR_FREE(progargv);
-
-    for (i = 0; progenv && progenv[i]; i++)
-        VIR_FREE(progenv[i]);
-    VIR_FREE(progenv);
+    virStringFreeList(progargv);
+    virStringFreeList(progenv);

     return def;
 }
@@ -11760,7 +11751,7 @@ cleanup:

 static int qemuParseProcFileStrings(int pid_value,
                                     const char *name,
-                                    const char ***list)
+                                    char ***list)
 {
     char *path = NULL;
     int ret = -1;
@@ -11769,7 +11760,6 @@ static int qemuParseProcFileStrings(int pid_value,
     char *tmp;
     size_t nstr = 0;
     char **str = NULL;
-    size_t i;

     if (virAsprintf(&path, "/proc/%d/%s", pid_value, name) < 0)
         goto cleanup;
@@ -11796,14 +11786,11 @@ static int qemuParseProcFileStrings(int pid_value,
     str[nstr-1] = NULL;

     ret = nstr-1;
-    *list = (const char **) str;
+    *list = str;

 cleanup:
-    if (ret < 0) {
-        for (i = 0; str && str[i]; i++)
-            VIR_FREE(str[i]);
-        VIR_FREE(str);
-    }
+    if (ret < 0)
+        virStringFreeList(str);
     VIR_FREE(data);
     VIR_FREE(path);
     return ret;
@@ -11817,11 +11804,10 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr qemuCaps,
                                         bool *monJSON)
 {
     virDomainDefPtr def = NULL;
-    const char **progargv = NULL;
-    const char **progenv = NULL;
+    char **progargv = NULL;
+    char **progenv = NULL;
     char *exepath = NULL;
     char *emulator;
-    size_t i;

     /* The parser requires /proc/pid, which only exists on platforms
      * like Linux where pid_t fits in int.  */
@@ -11848,11 +11834,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr qemuCaps,

 cleanup:
     VIR_FREE(exepath);
-    for (i = 0; progargv && progargv[i]; i++)
-        VIR_FREE(progargv[i]);
-    VIR_FREE(progargv);
-    for (i = 0; progenv && progenv[i]; i++)
-        VIR_FREE(progenv[i]);
-    VIR_FREE(progenv);
+    virStringFreeList(progargv);
+    virStringFreeList(progenv);
     return def;
 }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index a9854a3..2f248eb 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -205,13 +205,6 @@ int qemuNetworkPrepareDevices(virDomainDefPtr def);
  * NB: def->name can be NULL upon return and the caller
  * *must* decide how to fill in a name in this case
  */
-virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
-                                     virDomainXMLOptionPtr xmlopt,
-                                     const char **progenv,
-                                     const char **progargv,
-                                     char **pidfile,
-                                     virDomainChrSourceDefPtr *monConfig,
-                                     bool *monJSON);
 virDomainDefPtr qemuParseCommandLineString(virCapsPtr qemuCaps,
                                            virDomainXMLOptionPtr xmlopt,
                                            const char *args,
-- 
1.8.3.1




More information about the libvir-list mailing list