On Thu, May 16, 2013 at 08:40:53PM +0800, Osier Yang wrote:
virStorageVolDefParseXML: * Create "virStorageVolDefPtr def", and use ret to track the return value; frees the strings at "cleanup" label instead of freeing them in the middle.
virStorageVolDefFormat: * Use macro NULLSTR --- src/conf/storage_conf.c | 93 ++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 47 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index dd55d2c..431a8eb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -972,8 +972,8 @@ virStoragePoolDefParseNode(xmlDocPtr xml, virStoragePoolDefPtr def = NULL;
if (STRNEQ((const char *)root->name, "pool")) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("unknown root element for storage pool")); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("unknown root element for storage pool")); goto cleanup; }
@@ -1149,8 +1149,8 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
type = virStoragePoolTypeToString(def->type); if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unexpected pool type")); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected pool type")); goto cleanup; } virBufferAsprintf(&buf, "<pool type='%s'>\n", type); @@ -1214,8 +1214,8 @@ virStorageSize(const char *unit, unsigned long long *ret) { if (virStrToLong_ull(val, NULL, 10, ret) < 0) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("malformed capacity element")); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("malformed capacity element")); return -1; } /* off_t is signed, so you cannot create a file larger than 2**63 @@ -1230,64 +1230,62 @@ static virStorageVolDefPtr virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlXPathContextPtr ctxt) { - virStorageVolDefPtr ret; + virStorageVolDefPtr def; virStorageVolOptionsPtr options; char *allocation = NULL; char *capacity = NULL; char *unit = NULL; xmlNodePtr node; + virStorageVolDefPtr ret = NULL;
IMHO having 2 variables for the same thing is a bit odd. IOW, I think the current code is better than what you've done here. If you want to have unified cleanup, do it like cleanup: VIR_FREE(allocation); VIR_FREE(capacity); VIR_FREE(unit); return ret; error: virStorageVolDefFree(ret); ret = NULL; goto cleanup; 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 :|