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. Berrange"<berra...@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

Reply via email to