Re: [PATCH 05/14] Use a libvirt custom struct for virURIPtr

[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>

Instead of just typedef'ing the xmlURIPtr struct for virURIPtr,
use a custom libvirt struct. This allows us to fix various
problems with libxml2. This initially just fixes the query vs
query_raw handling problems.
---
  src/esx/esx_util.c         |    4 --
  src/hyperv/hyperv_util.c   |    4 --
  src/libvirt.c              |    5 +--
  src/remote/remote_driver.c |   16 +--------
  src/util/viruri.c          |   79 +++++++++++++++++++++++++++++++++++--------
  src/util/viruri.h          |   14 +++++++-
  6 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 7d4b908..67b07b7 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -62,11 +62,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
          return -1;
      }

-#ifdef HAVE_XMLURI_QUERY_RAW
-    queryParamSet = qparam_query_parse(uri->query_raw);
-#else
      queryParamSet = qparam_query_parse(uri->query);
-#endif

      if (queryParamSet == NULL) {
          goto cleanup;
diff --git a/src/hyperv/hyperv_util.c b/src/hyperv/hyperv_util.c
index 2e6a2d4..63c761b 100644
--- a/src/hyperv/hyperv_util.c
+++ b/src/hyperv/hyperv_util.c
@@ -54,11 +54,7 @@ hypervParseUri(hypervParsedUri **parsedUri, virURIPtr uri)
          return -1;
      }

-#ifdef HAVE_XMLURI_QUERY_RAW
-    queryParamSet = qparam_query_parse(uri->query_raw);
-#else
      queryParamSet = qparam_query_parse(uri->query);
-#endif

      if (queryParamSet == NULL) {
          goto cleanup;
diff --git a/src/libvirt.c b/src/libvirt.c
index f7590e0..fb7885f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1173,15 +1173,12 @@ do_open (const char *name,

          VIR_DEBUG("name \"%s\" to URI components:\n"
                    "  scheme %s\n"
-                  "  opaque %s\n"
-                  "  authority %s\n"
                    "  server %s\n"
                    "  user %s\n"
                    "  port %d\n"
                    "  path %s\n",
                    alias ? alias : name,
-                  NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->opaque),
-                  NULLSTR(ret->uri->authority), NULLSTR(ret->uri->server),
+                  NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->server),
                    NULLSTR(ret->uri->user), ret->uri->port,
                    NULLSTR(ret->uri->path));

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index c6c5809..9de966f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -417,15 +417,9 @@ doRemoteOpen (virConnectPtr conn,
       */
      struct qparam *var;
      int i;
-    char *query;

      if (conn->uri) {
-#ifdef HAVE_XMLURI_QUERY_RAW
-        query = conn->uri->query_raw;
-#else
-        query = conn->uri->query;
-#endif
-        vars = qparam_query_parse (query);
+        vars = qparam_query_parse (conn->uri->query);
          if (vars == NULL) goto failed;

          for (i = 0; i<  vars->n; i++) {
@@ -490,11 +484,7 @@ doRemoteOpen (virConnectPtr conn,
              } else {
                  virURI tmpuri = {
                      .scheme = conn->uri->scheme,
-#ifdef HAVE_XMLURI_QUERY_RAW
-                    .query_raw = qparam_get_query (vars),
-#else
                      .query = qparam_get_query (vars),
-#endif
                      .path = conn->uri->path,
                      .fragment = conn->uri->fragment,
                  };
@@ -507,11 +497,7 @@ doRemoteOpen (virConnectPtr conn,

                  name = virURIFormat(&tmpuri);

-#ifdef HAVE_XMLURI_QUERY_RAW
-                VIR_FREE(tmpuri.query_raw);
-#else
                  VIR_FREE(tmpuri.query);
-#endif

                  /* Restore transport scheme */
                  if (transport_str)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index bbd8742..d8618d1 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -36,15 +36,45 @@
  virURIPtr
  virURIParse(const char *uri)
  {
-    virURIPtr ret = xmlParseURI(uri);
+    xmlURIPtr xmluri;
+    virURIPtr ret = NULL;

-    if (!ret) {
+    xmluri = xmlParseURI(uri);
+
+    if (!uri) {

s/uri/xmluri/

          /* libxml2 does not tell us what failed. Grr :-( */
          virURIReportError(VIR_ERR_INTERNAL_ERROR,
                            "Unable to parse URI %s", uri);
          return NULL;
      }

+    if (VIR_ALLOC(ret)<  0)
+        goto no_memory;
+
+    if (xmluri->scheme&&
+        !(ret->scheme = strdup(xmluri->scheme)))
+        goto no_memory;
+    if (xmluri->server&&
+        !(ret->server = strdup(xmluri->server)))
+        goto no_memory;
+    ret->port = xmluri->port;
+    if (xmluri->path&&
+        !(ret->path = strdup(xmluri->path)))
+        goto no_memory;
+#ifdef HAVE_XMLURI_QUERY_RAW
+    if (xmluri->query_raw&&
+        !(ret->query = strdup(xmluri->query_raw)))
+        goto no_memory;
+#else
+    if (xmluri->query&&
+        !(ret->query = strdup(xmluri->query)))
+        goto no_memory;
+#endif
+    if (xmluri->fragment&&
+        !(ret->fragment = strdup(xmluri->fragment)))
+        goto no_memory;
+
+
      /* First check: does it even make sense to jump inside */
      if (ret->server != NULL&&
          ret->server[0] == '[') {
@@ -62,7 +92,15 @@ virURIParse(const char *uri)
           * the uri with xmlFreeURI() */
      }

+    xmlFreeURI(xmluri);
+
      return ret;
+
+no_memory:
+    virReportOOMError();
+    xmlFreeURI(xmluri);
+    virURIFree(ret);
+    return NULL;
  }

  /**
@@ -79,33 +117,37 @@ virURIParse(const char *uri)
  char *
  virURIFormat(virURIPtr uri)
  {
-    char *backupserver = NULL;
+    xmlURI xmluri;
      char *tmpserver = NULL;
      char *ret;

+    memset(&xmluri, 0, sizeof(xmluri));
+
+    xmluri.scheme = uri->scheme;
+    xmluri.server = uri->server;
+    xmluri.port = uri->port;
+    xmluri.path = uri->path;
+    xmluri.query = uri->query;
+    xmluri.fragment = uri->fragment;
+
      /* First check: does it make sense to do anything */
-    if (uri->server != NULL&&
-        strchr(uri->server, ':') != NULL) {
+    if (xmluri.server != NULL&&
+        strchr(xmluri.server, ':') != NULL) {

-        backupserver = uri->server;
-        if (virAsprintf(&tmpserver, "[%s]", uri->server)<  0)
+        if (virAsprintf(&tmpserver, "[%s]", xmluri.server)<  0)
              return NULL;

-        uri->server = tmpserver;
+        xmluri.server = tmpserver;
      }

-    ret = (char *) xmlSaveUri(uri);
+    ret = (char *)xmlSaveUri(&xmluri);
      if (!ret) {
          virReportOOMError();
          goto cleanup;
      }

  cleanup:
-    /* Put the fixed version back */
-    if (tmpserver) {
-        uri->server = backupserver;
-        VIR_FREE(tmpserver);
-    }
+    VIR_FREE(tmpserver);

No use of "cleanup" yet.


      return ret;
  }
@@ -119,5 +161,12 @@ cleanup:
   */
  void virURIFree(virURIPtr uri)
  {
-    xmlFreeURI(uri);
+    if (!uri)
+        return;
+
+    VIR_FREE(uri->scheme);
+    VIR_FREE(uri->server);
+    VIR_FREE(uri->path);
+    VIR_FREE(uri->query);
+    VIR_FREE(uri);
  }
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 5773dda..dd270de 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -13,8 +13,18 @@

  # include "internal.h"

-typedef xmlURI    virURI;
-typedef xmlURIPtr virURIPtr;
+typedef struct _virURI virURI;
+typedef virURI *virURIPtr;
+
+struct _virURI {
+    char *scheme;       /* the URI scheme */
+    char *server;       /* the server part */
+    char *user;         /* the user part */
+    int port;           /* the port number */
+    char *path;         /* the path string */
+    char *query;        /* the query string */
+    char *fragment;     /* the fragment string */
+};

  virURIPtr virURIParse(const char *uri)
      ATTRIBUTE_NONNULL(1);

ACK with s/uri/xmluri/

--
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]