[libvirt] [PATCH 06/14] Store parsed query parameters directly in the virURIPtr struct
Osier Yang
jyang at redhat.com
Thu Mar 22 07:33:29 UTC 2012
On 2012年03月21日 01:33, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at 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, 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
More information about the libvir-list
mailing list