Fix misc leaks in qparams code, support ; as param separator. Add test suite

diff --git a/ChangeLog b/ChangeLog
index 73f6b6924a014cf8269e9a70501b045b1561f316..6cdd50f3f7423e04fb642b46959fae14845db49a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+Thu May 22 19:47:29 EST 2008 Daniel P. Berrange <berrange@redhat.com>
+
+       * src/qparams.c: Support ; as a param separator. Misc memory
+       leaks
+       * tests/qparamtest.c, tests/Makefile.am: Add test suite for
+       qparams code
+
 Thu May 22 19:44:29 EST 2008 Daniel P. Berrange <berrange@redhat.com>
 
        * src/qemu_conf.c: Refactor qemudBuildCommandLine to use a

diff --git a/src/qparams.c b/src/qparams.c
index 88bf5c1571fd2c0ac041b7560a0c97a99dfb7abe..b5514a53df1df4675742e3336cbb1f792d3dfcf7 100644 (file)
--- a/src/qparams.c
+++ b/src/qparams.c
@@ -166,7 +166,7 @@ struct qparam_set *
 qparam_query_parse (const char *query)
 {
     struct qparam_set *ps;
-    const char *name, *value, *end, *eq;
+    const char *end, *eq;
 
     ps = new_qparam_set (0, NULL);
     if (!ps) return NULL;
@@ -174,9 +174,14 @@ qparam_query_parse (const char *query)
     if (!query || query[0] == '\0') return ps;
 
     while (*query) {
+        char *name = NULL, *value = NULL;
+
         /* Find the next separator, or end of the string. */
         end = strchr (query, '&');
-        if (!end) end = query + strlen (query);
+        if (!end)
+            end = strchr (query, ';');
+        if (!end)
+            end = query + strlen (query);
 
         /* Find the first '=' character between here and end. */
         eq = strchr (query, '=');
@@ -191,7 +196,6 @@ qparam_query_parse (const char *query)
          */
         else if (!eq) {
             name = xmlURIUnescapeString (query, end - query, NULL);
-            value = "";
             if (!name) goto out_of_memory;
         }
         /* Or if we have "name=" here (works around annoying
@@ -199,7 +203,6 @@ qparam_query_parse (const char *query)
          */
         else if (eq+1 == end) {
             name = xmlURIUnescapeString (query, eq - query, NULL);
-            value = "";
             if (!name) goto out_of_memory;
         }
         /* If the '=' character is at the beginning then we have
@@ -211,12 +214,23 @@ qparam_query_parse (const char *query)
         /* Otherwise it's "name=value". */
         else {
             name = xmlURIUnescapeString (query, eq - query, NULL);
+            if (!name)
+                goto out_of_memory;
             value = xmlURIUnescapeString (eq+1, end - (eq+1), NULL);
-            if (!name || !value) goto out_of_memory;
+            if (!value) {
+                VIR_FREE(name);
+                goto out_of_memory;
+            }
         }
 
         /* Append to the parameter set. */
-        if (append_qparam (ps, name, value) == -1) goto out_of_memory;
+        if (append_qparam (ps, name, value ? value : "") == -1) {
+            VIR_FREE(name);
+            VIR_FREE(value);
+            goto out_of_memory;
+        }
+        VIR_FREE(name);
+        VIR_FREE(value);
 
     next:
         query = end;

diff --git a/tests/.cvsignore b/tests/.cvsignore
index c698b972ab8040771a62d1bd9eca1b359da0c646..f09e6bb35988c04b4142ce38ed5afe87c248b8da 100644 (file)
--- a/tests/.cvsignore
+++ b/tests/.cvsignore
@@ -14,6 +14,7 @@ qemuxml2xmltest
 qemuxml2argvtest
 nodeinfotest
 statstest
+qparamtest
 *.gcda
 *.gcno
 

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c1688e798fe5d89627b5148ddad0701d93654940..214094f0a951a762b6cbac47f708a697dc7452b3 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -41,7 +41,7 @@ EXTRA_DIST =          \
 
 noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \
        reconnect xmconfigtest xencapstest qemuxml2argvtest qemuxml2xmltest \
-        nodeinfotest statstest
+        nodeinfotest statstest qparamtest
 
 test_scripts = \
        daemon-conf \
@@ -53,7 +53,7 @@ EXTRA_DIST += $(test_scripts)
 
 TESTS = xml2sexprtest sexpr2xmltest virshtest test_conf.sh xmconfigtest \
         xencapstest qemuxml2argvtest qemuxml2xmltest nodeinfotest \
-       statstest $(test_scripts)
+       statstest qparamtest $(test_scripts)
 if ENABLE_XEN_TESTS
   TESTS += reconnect
 endif
@@ -130,6 +130,10 @@ statstest_SOURCES = \
        statstest.c testutils.h testutils.c
 statstest_LDADD = $(LDADDS)
 
+qparamtest_SOURCES = \
+       qparamtest.c testutils.h testutils.c
+qparamtest_LDADD = $(LDADDS)
+
 reconnect_SOURCES = \
        reconnect.c
 reconnect_LDADD = $(LDADDS)

diff --git a/tests/qparamtest.c b/tests/qparamtest.c
new file mode 100644 (file)
index 0000000..cec3938
--- /dev/null
+++ b/tests/qparamtest.c
@@ -0,0 +1,224 @@
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "testutils.h"
+#include "qparams.h"
+#include "util.h"
+
+struct qparamParseDataEntry {
+    const char *name;
+    const char *value;
+};
+
+struct qparamParseData {
+    const char *queryIn;
+    const char *queryOut;
+    int nparams;
+    const struct qparamParseDataEntry *params;
+};
+
+static int
+qparamParseTest(const void *data)
+{
+    const struct qparamParseData *expect = data;
+    struct qparam_set *actual = qparam_query_parse(expect->queryIn);
+    int ret = -1, i;
+    if (!actual)
+        return -1;
+
+    if (actual->n != expect->nparams)
+        goto fail;
+
+    for (i = 0 ; i < actual->n ; i++) {
+        if (!STREQ(expect->params[i].name,
+                   actual->p[i].name))
+            goto fail;
+        if (!STREQ(expect->params[i].value,
+                   actual->p[i].value))
+            goto fail;
+    }
+
+    ret = 0;
+
+fail:
+    free_qparam_set(actual);
+    return ret;
+}
+
+static int
+qparamFormatTest(const void *data)
+{
+    const struct qparamParseData *expect = data;
+    struct qparam_set *actual = qparam_query_parse(expect->queryIn);
+    char *output = NULL;
+    int ret = -1;
+
+    if (!actual)
+        return -1;
+
+    output = qparam_get_query(actual);
+    if (!output)
+        goto fail;
+
+    if (!STREQ(output, expect->queryOut))
+        goto fail;
+
+    ret = 0;
+
+fail:
+    free(output);
+    free_qparam_set(actual);
+    return ret;
+}
+
+static int
+qparamBuildTest(const void *data)
+{
+    const struct qparamParseData *expect = data;
+    struct qparam_set *actual = new_qparam_set(0, NULL);
+    int ret = -1, i;
+    if (!actual)
+        return -1;
+
+    for (i = 0 ; i < expect->nparams ; i++) {
+        if (append_qparam(actual,
+                          expect->params[i].name,
+                          expect->params[i].value) < 0)
+            goto fail;
+    }
+
+    if (actual->n != expect->nparams)
+        goto fail;
+
+    for (i = 0 ; i < actual->n ; i++) {
+        if (!STREQ(expect->params[i].name,
+                   actual->p[i].name))
+            goto fail;
+        if (!STREQ(expect->params[i].value,
+                   actual->p[i].value))
+            goto fail;
+    }
+
+    ret = 0;
+
+fail:
+    free_qparam_set(actual);
+    return ret;
+}
+
+
+static int
+qparamTestNewVargs(const void *data ATTRIBUTE_UNUSED)
+{
+    struct qparam_set *actual = new_qparam_set(0, "foo", "one", "bar", "two", NULL);
+    int ret = -1;
+    if (!actual)
+        return -1;
+
+    if (actual->n != 2)
+        goto fail;
+
+    if (!STREQ(actual->p[0].name, "foo"))
+        goto fail;
+
+    if (!STREQ(actual->p[0].value, "one"))
+        goto fail;
+
+    if (!STREQ(actual->p[1].name, "bar"))
+        goto fail;
+
+    if (!STREQ(actual->p[1].value, "two"))
+        goto fail;
+
+    ret = 0;
+
+fail:
+    free_qparam_set(actual);
+    return ret;
+}
+
+static int
+qparamTestAddVargs(const void *data ATTRIBUTE_UNUSED)
+{
+    struct qparam_set *actual = new_qparam_set(0, NULL);
+    int ret = -1;
+    if (!actual)
+        return -1;
+
+    if (append_qparams(actual,  "foo", "one", "bar", "two", NULL) < 0)
+        goto fail;
+
+    if (actual->n != 2)
+        goto fail;
+
+    if (!STREQ(actual->p[0].name, "foo"))
+        goto fail;
+
+    if (!STREQ(actual->p[0].value, "one"))
+        goto fail;
+
+    if (!STREQ(actual->p[1].name, "bar"))
+        goto fail;
+
+    if (!STREQ(actual->p[1].value, "two"))
+        goto fail;
+
+    ret = 0;
+
+fail:
+    free_qparam_set(actual);
+    return ret;
+}
+
+static const struct qparamParseDataEntry const params1[] = { { "foo", "one" }, { "bar", "two" } };
+static const struct qparamParseDataEntry const params2[] = { { "foo", "one" }, { "foo", "two" } };
+static const struct qparamParseDataEntry const params3[] = { { "foo", "&one" }, { "bar", "&two" } };
+static const struct qparamParseDataEntry const params4[] = { { "foo", "" } };
+static const struct qparamParseDataEntry const params5[] = { { "foo", "one two" } };
+static const struct qparamParseDataEntry const params6[] = { { "foo", "one" } };
+
+int
+main(void)
+{
+    int ret = 0;
+
+#define DO_TEST(queryIn,queryOut,params)                                \
+    do {                                                                \
+        struct qparamParseData info = {                                 \
+            queryIn,                                                    \
+            queryOut ? queryOut : queryIn,                              \
+            sizeof(params)/sizeof(params[0]),                           \
+            params };                                                   \
+        if (virtTestRun("Parse " queryIn,                               \
+                        1, qparamParseTest, &info) < 0)                 \
+            ret = -1;                                                   \
+        if (virtTestRun("Format " queryIn,                              \
+                        1, qparamFormatTest, &info) < 0)                \
+            ret = -1;                                                   \
+        if (virtTestRun("Build " queryIn,                               \
+                        1, qparamBuildTest, &info) < 0)                 \
+            ret = -1;                                                   \
+    } while (0)
+
+
+    DO_TEST("foo=one&bar=two", NULL, params1);
+    DO_TEST("foo=one&foo=two", NULL, params2);
+    DO_TEST("foo=one&&foo=two", "foo=one&foo=two", params2);
+    DO_TEST("foo=one;foo=two", "foo=one&foo=two", params2);
+    DO_TEST("foo", "foo=", params4);
+    DO_TEST("foo=", NULL, params4);
+    DO_TEST("foo=&", "foo=", params4);
+    DO_TEST("foo=&&", "foo=", params4);
+    DO_TEST("foo=one%20two", NULL, params5);
+    DO_TEST("=bogus&foo=one", "foo=one", params6);
+
+    if (virtTestRun("New vargs", 1, qparamTestNewVargs, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Add vargs", 1, qparamTestAddVargs, NULL) < 0)
+        ret = -1;
+
+    exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}