Re: [libvirt] [PATCH 04/14] Centralize error reporting for URI parsing/formatting problems

2012-03-23 Thread Daniel P. Berrange 
On Thu, Mar 22, 2012 at 12:03:11PM +0800, Osier Yang wrote:
 On 2012年03月21日 01:33, Daniel P. Berrange wrote:
 From: Daniel P. Berrangeberra...@redhat.com
 @@ -70,8 +84,7 @@ virURIFormat(virURIPtr uri)
   char *ret;
 
   /* First check: does it make sense to do anything */
 -if (uri != NULL
 -uri-server != NULL
 +if (uri-server != NULL
   strchr(uri-server, ':') != NULL) {
 
   backupserver = uri-server;
 @@ -82,7 +95,12 @@ virURIFormat(virURIPtr uri)
   }
 
   ret = (char *) xmlSaveUri(uri);
 +if (!ret) {
 +virReportOOMError();
 +goto cleanup;
 +}
 
 +cleanup:
 
 The cleanup label doesn't make any sense. Or it's for follow up
 pacthes use? but it should be together with the follow up patch
 if so.

I think it is preferrable to always have an explicit cleanup:
statement in this scenario, rather than relying on fallthrough.
It avoids future code additionals introducing cleanup bugs.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 04/14] Centralize error reporting for URI parsing/formatting problems

2012-03-21 Thread Osier Yang 

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

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

Move error reporting out of the callers, into virURIParse
and virURIFormat, to get consistency.

* include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_URI
* src/util/viruri.c, src/util/viruri.h: Add error reporting
* src/esx/esx_driver.c, src/libvirt.c, src/libxl/libxl_driver.c,
   src/lxc/lxc_driver.c, src/openvz/openvz_driver.c,
   src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
   src/remote/remote_driver.c, src/uml/uml_driver.c,
   src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xen/xen_driver.c,
   src/xen/xend_internal.c, tests/viruritest.c: Remove error
   reporting
---
  include/libvirt/virterror.h |1 +
  src/esx/esx_driver.c|6 +-
  src/libvirt.c   |   16 
  src/libxl/libxl_driver.c|5 +
  src/lxc/lxc_driver.c|5 +
  src/openvz/openvz_driver.c  |5 +
  src/qemu/qemu_driver.c  |9 +++--
  src/qemu/qemu_migration.c   |5 +
  src/remote/remote_driver.c  |   18 --
  src/uml/uml_driver.c|9 +++--
  src/util/virterror.c|3 +++
  src/util/viruri.c   |   26 ++
  src/util/viruri.h   |6 --
  src/vbox/vbox_tmpl.c|   10 +++---
  src/vmx/vmx.c   |6 +-
  src/xen/xen_driver.c|5 +
  src/xen/xend_internal.c |8 +++-
  tests/viruritest.c  |8 ++--
  18 files changed, 63 insertions(+), 88 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 38ac15e..c8ec8d4 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -85,6 +85,7 @@ typedef enum {
  VIR_FROM_LOCKING = 42,/* Error from lock manager */
  VIR_FROM_HYPERV = 43, /* Error from Hyper-V driver */
  VIR_FROM_CAPABILITIES = 44, /* Error from capabilities */
+VIR_FROM_URI = 45,  /* Error from URI handling */
  } virErrorDomain;


diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index dbf95f4..7e41fa3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3977,12 +3977,8 @@ esxDomainMigratePerform(virDomainPtr domain,
  }

  /* Parse migration URI */
-parsedUri = virURIParse(uri);
-
-if (parsedUri == NULL) {
-virReportOOMError();
+if (!(parsedUri = virURIParse(uri)))
  return -1;
-}

  if (parsedUri-scheme == NULL || STRCASENEQ(parsedUri-scheme, 
vpxmigr)) {
  ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
diff --git a/src/libvirt.c b/src/libvirt.c
index f58623a..f7590e0 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1166,11 +1166,7 @@ do_open (const char *name,
  virConnectOpenResolveURIAlias(conf, name,alias)  0)
  goto failed;

-ret-uri = virURIParse (alias ? alias : name);
-if (!ret-uri) {
-virLibConnError(VIR_ERR_INVALID_ARG,
-_(could not parse connection URI %s),
-alias ? alias : name);
+if (!(ret-uri = virURIParse (alias ? alias : name))) {
  VIR_FREE(alias);
  goto failed;
  }
@@ -1771,11 +1767,9 @@ virConnectGetURI (virConnectPtr conn)
  return NULL;
  }

-name = virURIFormat(conn-uri);
-if (!name) {
-virReportOOMError();
+if (!(name = virURIFormat(conn-uri)))
  goto error;
-}
+
  return name;

  error:
@@ -5062,9 +5056,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
  return -1;
  }

-tempuri = virURIParse(dconnuri);
-if (!tempuri) {
-virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+if (!(tempuri = virURIParse(dconnuri))) {
  virDispatchError(domain-conn);
  return -1;
  }
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 177b218..eccd198 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1044,11 +1044,8 @@ libxlOpen(virConnectPtr conn,
  if (libxl_driver == NULL)
  return VIR_DRV_OPEN_DECLINED;

-conn-uri = virURIParse(xen:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(xen:///)))
  return VIR_DRV_OPEN_ERROR;
-}
  } else {
  /* Only xen scheme */
  if (conn-uri-scheme == NULL || STRNEQ(conn-uri-scheme, xen))
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3af8084..29842a5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -140,11 +140,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn,
  if (lxc_driver == NULL)
  return VIR_DRV_OPEN_DECLINED;

-conn-uri = virURIParse(lxc:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(lxc:///)))
  return VIR_DRV_OPEN_ERROR;
-}
  } else {
  if

[libvirt] [PATCH 04/14] Centralize error reporting for URI parsing/formatting problems

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

Move error reporting out of the callers, into virURIParse
and virURIFormat, to get consistency.

* include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_URI
* src/util/viruri.c, src/util/viruri.h: Add error reporting
* src/esx/esx_driver.c, src/libvirt.c, src/libxl/libxl_driver.c,
  src/lxc/lxc_driver.c, src/openvz/openvz_driver.c,
  src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
  src/remote/remote_driver.c, src/uml/uml_driver.c,
  src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xen/xen_driver.c,
  src/xen/xend_internal.c, tests/viruritest.c: Remove error
  reporting
---
 include/libvirt/virterror.h |1 +
 src/esx/esx_driver.c|6 +-
 src/libvirt.c   |   16 
 src/libxl/libxl_driver.c|5 +
 src/lxc/lxc_driver.c|5 +
 src/openvz/openvz_driver.c  |5 +
 src/qemu/qemu_driver.c  |9 +++--
 src/qemu/qemu_migration.c   |5 +
 src/remote/remote_driver.c  |   18 --
 src/uml/uml_driver.c|9 +++--
 src/util/virterror.c|3 +++
 src/util/viruri.c   |   26 ++
 src/util/viruri.h   |6 --
 src/vbox/vbox_tmpl.c|   10 +++---
 src/vmx/vmx.c   |6 +-
 src/xen/xen_driver.c|5 +
 src/xen/xend_internal.c |8 +++-
 tests/viruritest.c  |8 ++--
 18 files changed, 63 insertions(+), 88 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 38ac15e..c8ec8d4 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -85,6 +85,7 @@ typedef enum {
 VIR_FROM_LOCKING = 42, /* Error from lock manager */
 VIR_FROM_HYPERV = 43,  /* Error from Hyper-V driver */
 VIR_FROM_CAPABILITIES = 44, /* Error from capabilities */
+VIR_FROM_URI = 45,  /* Error from URI handling */
 } virErrorDomain;
 
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index dbf95f4..7e41fa3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3977,12 +3977,8 @@ esxDomainMigratePerform(virDomainPtr domain,
 }
 
 /* Parse migration URI */
-parsedUri = virURIParse(uri);
-
-if (parsedUri == NULL) {
-virReportOOMError();
+if (!(parsedUri = virURIParse(uri)))
 return -1;
-}
 
 if (parsedUri-scheme == NULL || STRCASENEQ(parsedUri-scheme, vpxmigr)) 
{
 ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
diff --git a/src/libvirt.c b/src/libvirt.c
index f58623a..f7590e0 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1166,11 +1166,7 @@ do_open (const char *name,
 virConnectOpenResolveURIAlias(conf, name, alias)  0)
 goto failed;
 
-ret-uri = virURIParse (alias ? alias : name);
-if (!ret-uri) {
-virLibConnError(VIR_ERR_INVALID_ARG,
-_(could not parse connection URI %s),
-alias ? alias : name);
+if (!(ret-uri = virURIParse (alias ? alias : name))) {
 VIR_FREE(alias);
 goto failed;
 }
@@ -1771,11 +1767,9 @@ virConnectGetURI (virConnectPtr conn)
 return NULL;
 }
 
-name = virURIFormat(conn-uri);
-if (!name) {
-virReportOOMError();
+if (!(name = virURIFormat(conn-uri)))
 goto error;
-}
+
 return name;
 
 error:
@@ -5062,9 +5056,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
 return -1;
 }
 
-tempuri = virURIParse(dconnuri);
-if (!tempuri) {
-virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+if (!(tempuri = virURIParse(dconnuri))) {
 virDispatchError(domain-conn);
 return -1;
 }
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 177b218..eccd198 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1044,11 +1044,8 @@ libxlOpen(virConnectPtr conn,
 if (libxl_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;
 
-conn-uri = virURIParse(xen:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(xen:///)))
 return VIR_DRV_OPEN_ERROR;
-}
 } else {
 /* Only xen scheme */
 if (conn-uri-scheme == NULL || STRNEQ(conn-uri-scheme, xen))
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3af8084..29842a5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -140,11 +140,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn,
 if (lxc_driver == NULL)
 return VIR_DRV_OPEN_DECLINED;
 
-conn-uri = virURIParse(lxc:///);
-if (!conn-uri) {
-virReportOOMError();
+if (!(conn-uri = virURIParse(lxc:///)))
 return VIR_DRV_OPEN_ERROR;
-}
 } else {
 if (conn-uri-scheme == NULL ||
 STRNEQ(conn-uri-scheme, lxc))
diff --git