[libvirt] [PATCH 4/5] storage_conf: Left fixes or improvements for storage_conf.c
Daniel P. Berrange
berrange at redhat.com
Mon May 20 11:33:19 UTC 2013
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 :|
More information about the libvir-list
mailing list