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

2012-03-23 Thread Eric Blake 
On 03/20/2012 11:33 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 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(-)

This patch does not round-trip queries that include % escapes.  The old
qparamtest.c tried to roundtrip foo=one%20two (the literal value of 'one
two' when not URI-encoded); but now virURIFormat re-escapes the % into
one%2520two.  I caught this with my attempt to enhance viruritest to the
same level as qparamtest, but didn't want to spend any further time in
fixing viruri.c.

https://www.redhat.com/archives/libvir-list/2012-March/msg01043.html

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-03-22 Thread Osier Yang 

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

From: Daniel P. Berrangeberra...@redhat.com

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,

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

2012-03-22 Thread Osier Yang 

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

From: Daniel P. Berrangeberra...@redhat.com

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)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int i, amp = 0;
+
+for (i = 0; i  uri-paramsCount; ++i) {
+if (!uri-params[i].ignore) {
+if (amp) virBufferAddChar

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

2012-03-20 Thread Daniel P. Berrange 
From: Daniel P. Berrange berra...@redhat.com

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)
+{
+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,