[libvirt] [PATCH 4/5] storage_conf: Left fixes or improvements for storage_conf.c
Osier Yang
jyang at redhat.com
Thu May 16 12:40:53 UTC 2013
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;
options = virStorageVolOptionsForPoolType(pool->type);
if (options == NULL)
return NULL;
- if (VIR_ALLOC(ret) < 0) {
+ if (VIR_ALLOC(def) < 0) {
virReportOOMError();
return NULL;
}
- ret->name = virXPathString("string(./name)", ctxt);
- if (ret->name == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing volume name element"));
+ def->name = virXPathString("string(./name)", ctxt);
+ if (def->name == NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing volume name element"));
goto cleanup;
}
/* Auto-generated so deliberately ignore */
- /* ret->key = virXPathString("string(./key)", ctxt); */
+ /* def->key = virXPathString("string(./key)", ctxt); */
capacity = virXPathString("string(./capacity)", ctxt);
unit = virXPathString("string(./capacity/@unit)", ctxt);
if (capacity == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing capacity element"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing capacity element"));
goto cleanup;
}
- if (virStorageSize(unit, capacity, &ret->capacity) < 0)
+ if (virStorageSize(unit, capacity, &def->capacity) < 0)
goto cleanup;
- VIR_FREE(capacity);
VIR_FREE(unit);
allocation = virXPathString("string(./allocation)", ctxt);
if (allocation) {
unit = virXPathString("string(./allocation/@unit)", ctxt);
- if (virStorageSize(unit, allocation, &ret->allocation) < 0)
+ if (virStorageSize(unit, allocation, &def->allocation) < 0)
goto cleanup;
- VIR_FREE(allocation);
- VIR_FREE(unit);
} else {
- ret->allocation = ret->capacity;
+ def->allocation = def->capacity;
}
- ret->target.path = virXPathString("string(./target/path)", ctxt);
+ def->target.path = virXPathString("string(./target/path)", ctxt);
if (options->formatFromString) {
char *format = virXPathString("string(./target/format/@type)", ctxt);
if (format == NULL)
- ret->target.format = options->defaultFormat;
+ def->target.format = options->defaultFormat;
else
- ret->target.format = (options->formatFromString)(format);
+ def->target.format = (options->formatFromString)(format);
- if (ret->target.format < 0) {
+ if (def->target.format < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("unknown volume format type %s"), format);
VIR_FREE(format);
@@ -1296,28 +1294,28 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(format);
}
- if (virStorageDefParsePerms(ctxt, &ret->target.perms,
+ if (virStorageDefParsePerms(ctxt, &def->target.perms,
"./target/permissions",
DEFAULT_VOL_PERM_MODE) < 0)
goto cleanup;
node = virXPathNode("./target/encryption", ctxt);
if (node != NULL) {
- ret->target.encryption = virStorageEncryptionParseNode(ctxt->doc,
+ def->target.encryption = virStorageEncryptionParseNode(ctxt->doc,
node);
- if (ret->target.encryption == NULL)
+ if (def->target.encryption == NULL)
goto cleanup;
}
- ret->backingStore.path = virXPathString("string(./backingStore/path)", ctxt);
+ def->backingStore.path = virXPathString("string(./backingStore/path)", ctxt);
if (options->formatFromString) {
char *format = virXPathString("string(./backingStore/format/@type)", ctxt);
if (format == NULL)
- ret->backingStore.format = options->defaultFormat;
+ def->backingStore.format = options->defaultFormat;
else
- ret->backingStore.format = (options->formatFromString)(format);
+ def->backingStore.format = (options->formatFromString)(format);
- if (ret->backingStore.format < 0) {
+ if (def->backingStore.format < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("unknown volume format type %s"), format);
VIR_FREE(format);
@@ -1326,19 +1324,19 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(format);
}
- if (virStorageDefParsePerms(ctxt, &ret->backingStore.perms,
+ if (virStorageDefParsePerms(ctxt, &def->backingStore.perms,
"./backingStore/permissions",
DEFAULT_VOL_PERM_MODE) < 0)
goto cleanup;
- return ret;
-
+ ret = def;
cleanup:
VIR_FREE(allocation);
VIR_FREE(capacity);
VIR_FREE(unit);
- virStorageVolDefFree(ret);
- return NULL;
+ if (!ret)
+ virStorageVolDefFree(ret);
+ return ret;
}
virStorageVolDefPtr
@@ -1350,8 +1348,8 @@ virStorageVolDefParseNode(virStoragePoolDefPtr pool,
virStorageVolDefPtr def = NULL;
if (STRNEQ((const char *)root->name, "volume")) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("unknown root element for storage vol"));
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("unknown root element for storage vol"));
goto cleanup;
}
@@ -1481,7 +1479,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
virBufferAddLit(&buf, "<volume>\n");
virBufferAsprintf(&buf," <name>%s</name>\n", def->name);
- virBufferAsprintf(&buf," <key>%s</key>\n", def->key ? def->key : "(null)");
+ virBufferAsprintf(&buf," <key>%s</key>\n", NULLSTR(def->key));
virBufferAddLit(&buf, " <source>\n");
if (def->source.nextent) {
@@ -1658,8 +1656,8 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
}
if (virMutexInit(&pool->lock) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot initialize mutex"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot initialize mutex"));
VIR_FREE(pool);
return NULL;
}
@@ -1694,7 +1692,8 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
if (!virFileMatchesNameSuffix(file, def->name, ".xml")) {
virReportError(VIR_ERR_XML_ERROR,
- _("Storage pool config filename '%s' does not match pool name '%s'"),
+ _("Storage pool config filename '%s' "
+ "does not match pool name '%s'"),
path, def->name);
virStoragePoolDefFree(def);
return NULL;
@@ -1807,8 +1806,8 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
}
if (!(xml = virStoragePoolDefFormat(def))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("failed to generate XML"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to generate XML"));
return -1;
}
@@ -1870,8 +1869,8 @@ virStoragePoolSourceListFormat(virStoragePoolSourceListPtr 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;
}
--
1.8.1.4
More information about the libvir-list
mailing list