Re: [PATCH 06/14] Store parsed query parameters directly in the virURIPtr struct

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

 

On 2012年03月21日 01:33, Daniel P. Berrange wrote:

From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>

Avoid the need for each driver to parse query parameters itself
by storing them directly in the virURIPtr struct. The parsing
code is a copy of that from src/util/qparams.c  The latter will
be removed in a later patch

* src/util/viruri.h: Add query params to virURIPtr
* src/util/viruri.c: Parse query parameters when creating virURIPtr
* tests/viruritest.c: Expand test to cover params
---
  src/libvirt_private.syms |    1 +
  src/util/viruri.c        |  139 ++++++++++++++++++++++++++++++++++++++++++++++
  src/util/viruri.h        |   15 +++++
  tests/viruritest.c       |   46 +++++++++++++---
  4 files changed, 193 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7cd6a96..49fb2ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1471,6 +1471,7 @@ virTypedParameterAssign;
  # viruri.h
  virURIFree;
  virURIFormat;
+virURIFormatQuery;
  virURIParse;


diff --git a/src/util/viruri.c b/src/util/viruri.c
index d8618d1..f5adca5 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -13,6 +13,7 @@
  #include "memory.h"
  #include "util.h"
  #include "virterror_internal.h"
+#include "buf.h"

  #define VIR_FROM_THIS VIR_FROM_URI

@@ -21,6 +22,117 @@
                           __FUNCTION__, __LINE__, __VA_ARGS__)


+static int
+virURIParamAppend(virURIPtr uri,
+                  const char *name,
+                  const char *value)
+{
+    char *pname = NULL;
+    char *pvalue = NULL;
+
+    if (!(pname = strdup(name)))
+        goto no_memory;
+    if (!(pvalue = strdup (value)))
+        goto no_memory;
+
+    if (VIR_RESIZE_N(uri->params, uri->paramsAlloc, uri->paramsCount, 1)<  0)
+        goto no_memory;
+
+    uri->params[uri->paramsCount].name = pname;
+    uri->params[uri->paramsCount].value = pvalue;
+    uri->params[uri->paramsCount].ignore = 0;
+    uri->paramsCount++;
+
+    return 0;
+
+no_memory:
+    VIR_FREE(pname);
+    VIR_FREE(pvalue);
+    virReportOOMError();
+    return -1;
+}
+
+
+static int
+virURIParseParams(virURIPtr uri)
+{
+    const char *end, *eq;
+    const char *query = uri->query;
+
+    if (!query || query[0] == '\0')
+        return 0;
+
+    while (*query) {
+        char *name = NULL, *value = NULL;
+
+        /* Find the next separator, or end of the string. */
+        end = strchr (query, '&');
+        if (!end)
+            end = strchr (query, ';');
+        if (!end)
+            end = query + strlen (query);
+
+        /* Find the first '=' character between here and end. */
+        eq = strchr (query, '=');
+        if (eq&&  eq>= end) eq = NULL;
+
+        /* Empty section (eg. "&&"). */
+        if (end == query)
+            goto next;
+
+        /* If there is no '=' character, then we have just "name"
+         * and consistent with CGI.pm we assume value is "".
+         */
+        else if (!eq) {
+            name = xmlURIUnescapeString (query, end - query, NULL);
+            if (!name) goto no_memory;
+        }
+        /* Or if we have "name=" here (works around annoying
+         * problem when calling xmlURIUnescapeString with len = 0).
+         */
+        else if (eq+1 == end) {
+            name = xmlURIUnescapeString (query, eq - query, NULL);
+            if (!name) goto no_memory;
+        }
+        /* If the '=' character is at the beginning then we have
+         * "=value" and consistent with CGI.pm we _ignore_ this.
+         */
+        else if (query == eq)
+            goto next;
+
+        /* Otherwise it's "name=value". */
+        else {
+            name = xmlURIUnescapeString (query, eq - query, NULL);
+            if (!name)
+                goto no_memory;
+            value = xmlURIUnescapeString (eq+1, end - (eq+1), NULL);
+            if (!value) {
+                VIR_FREE(name);
+                goto no_memory;
+            }
+        }
+
+        /* Append to the parameter set. */
+        if (virURIParamAppend(uri, name, value ? value : "")<  0) {
+            VIR_FREE(name);
+            VIR_FREE(value);
+            goto no_memory;
+        }
+        VIR_FREE(name);
+        VIR_FREE(value);
+
+    next:
+        query = end;
+        if (*query) query ++; /* skip '&' separator */
+    }
+
+    return 0;
+
+ no_memory:
+    virReportOOMError();
+    return -1;
+}
+
  /**
   * virURIParse:
   * @uri: URI to parse
@@ -92,12 +204,16 @@ virURIParse(const char *uri)
           * the uri with xmlFreeURI() */
      }

+    if (virURIParseParams(ret)<  0)
+        goto error;
+
      xmlFreeURI(xmluri);

      return ret;

  no_memory:
      virReportOOMError();
+error:
      xmlFreeURI(xmluri);
      virURIFree(ret);
      return NULL;
@@ -153,6 +269,29 @@ cleanup:
  }


+char *virURIFormatQuery(virURIPtr uri)


Should we name the function as virURIFormatParams instead? Per
params is used in the context everywhere, and virURIParseParams.


+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    int i, amp = 0;
+
+    for (i = 0; i<  uri->paramsCount; ++i) {
+        if (!uri->params[i].ignore) {
+            if (amp) virBufferAddChar (&buf, '&');
+            virBufferStrcat (&buf, uri->params[i].name, "=", NULL);
+            virBufferURIEncodeString (&buf, uri->params[i].value);
+            amp = 1;
+        }
+    }
+
+    if (virBufferError(&buf)) {
+        virBufferFreeAndReset(&buf);
+        virReportOOMError();
+        return NULL;
+    }
+
+    return virBufferContentAndReset(&buf);
+}
+
  /**
   * virURIFree:
   * @uri: uri to free
diff --git a/src/util/viruri.h b/src/util/viruri.h
index dd270de..6fe0b2e 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -16,6 +16,15 @@
  typedef struct _virURI virURI;
  typedef virURI *virURIPtr;

+typedef struct _virURIParam virURIParam;
+typedef virURIParam *virURIParamPtr;
+
+struct _virURIParam {
+    char *name;  /* Name (unescaped). */
+    char *value; /* Value (unescaped). */
+    bool ignore; /* Ignore this field in qparam_get_query */


s/qparam_get_query/virURIFormatParams/


+};
+
  struct _virURI {
      char *scheme;       /* the URI scheme */
      char *server;       /* the server part */
@@ -24,6 +33,10 @@ struct _virURI {
      char *path;         /* the path string */
      char *query;        /* the query string */
      char *fragment;     /* the fragment string */
+
+    size_t paramsCount;
+    size_t paramsAlloc;
+    virURIParamPtr params;
  };

  virURIPtr virURIParse(const char *uri)
@@ -31,6 +44,8 @@ virURIPtr virURIParse(const char *uri)
  char *virURIFormat(virURIPtr uri)
      ATTRIBUTE_NONNULL(1);

+char *virURIFormatQuery(virURIPtr uri);
+
  void virURIFree(virURIPtr uri);

  #endif /* __VIR_URI_H__ */
diff --git a/tests/viruritest.c b/tests/viruritest.c
index fea5ddd..1d27697 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -41,6 +41,7 @@ struct URIParseData {
      const char *path;
      const char *query;
      const char *fragment;
+    virURIParamPtr params;
  };

  static int testURIParse(const void *args)
@@ -49,6 +50,7 @@ static int testURIParse(const void *args)
      virURIPtr uri = NULL;
      const struct URIParseData *data = args;
      char *uristr;
+    size_t i;

      if (!(uri = virURIParse(data->uri)))
          goto cleanup;
@@ -98,6 +100,29 @@ static int testURIParse(const void *args)
          goto cleanup;
      }

+    for (i = 0 ; data->params&&  data->params[i].name&&  i<  uri->paramsCount ; i++) {
+        if (!STREQ_NULLABLE(data->params[i].name, uri->params[i].name)) {
+            VIR_DEBUG("Expected param name %zu '%s', actual '%s'",
+                      i, data->params[i].name, uri->params[i].name);
+            goto cleanup;
+        }
+        if (!STREQ_NULLABLE(data->params[i].value, uri->params[i].value)) {
+            VIR_DEBUG("Expected param value %zu '%s', actual '%s'",
+                      i, data->params[i].value, uri->params[i].value);
+            goto cleanup;
+        }
+    }
+    if (data->params&&  data->params[i].name) {
+        VIR_DEBUG("Missing parameter %zu %s=%s",
+                  i, data->params[i].name, data->params[i].value);
+        goto cleanup;
+    }
+    if (i != uri->paramsCount) {
+        VIR_DEBUG("Unexpected parameter %zu %s=%s",
+                  i, uri->params[i].name, uri->params[i].value);
+        goto cleanup;
+    }
+
      ret = 0;
  cleanup:
      VIR_FREE(uristr);
@@ -113,21 +138,26 @@ mymain(void)

      signal(SIGPIPE, SIG_IGN);

-#define TEST_PARSE(uri, scheme, server, port, path, query, fragment)    \
+#define TEST_PARSE(uri, scheme, server, port, path, query, fragment, params) \
      do  {                                                               \
          const        struct URIParseData data = {                       \
-            uri, scheme, server, port, path, query, fragment            \
+            uri, scheme, server, port, path, query, fragment, params    \
          };                                                              \
          if (virtTestRun("Test IPv6 " # uri,  1, testURIParse,&data)<  0) \
              ret = -1;                                                   \
      } while (0)

-    TEST_PARSE("test://example.com", "test", "example.com", 0, NULL, NULL, NULL);
-    TEST_PARSE("test://example.com:123", "test", "example.com", 123, NULL, NULL, NULL);
-    TEST_PARSE("test://example.com:123/system?name=value#foo", "test", "example.com", 123, "/system", "name=value", "foo");
-    TEST_PARSE("test://127.0.0.1:123/system", "test", "127.0.0.1", 123, "/system", NULL, NULL);
-    TEST_PARSE("test://[::1]:123/system", "test", "::1", 123, "/system", NULL, NULL);
-    TEST_PARSE("test://[2001:41c8:1:4fd4::2]:123/system", "test", "2001:41c8:1:4fd4::2", 123, "/system", NULL, NULL);
+    virURIParam params[] = {
+        { (char*)"name", (char*)"value" },
+        { NULL, NULL },
+    };
+
+    TEST_PARSE("test://example.com", "test", "example.com", 0, NULL, NULL, NULL, NULL);
+    TEST_PARSE("test://example.com:123", "test", "example.com", 123, NULL, NULL, NULL, NULL);
+    TEST_PARSE("test://example.com:123/system?name=value#foo", "test", "example.com", 123, "/system", "name=value", "foo", params);
+    TEST_PARSE("test://127.0.0.1:123/system", "test", "127.0.0.1", 123, "/system", NULL, NULL, NULL);
+    TEST_PARSE("test://[::1]:123/system", "test", "::1", 123, "/system", NULL, NULL, NULL);
+    TEST_PARSE("test://[2001:41c8:1:4fd4::2]:123/system", "test", "2001:41c8:1:4fd4::2", 123, "/system", NULL, NULL, NULL);

      return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
  }


Others look fine, ACK with the function name changed
and copy & paste comment fixed.

Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]