Re: [libvirt] [PATCH 03/15] conf: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageVolDef

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. For virStorageVolDefParseXML use the @def/@ret similarly as other methods. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; ---

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6099c64b26..83ca379217 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1168,7 +1168,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlXPathContextPtr ctxt, unsigned int flags) { - virStorageVolDefPtr ret; + VIR_AUTOPTR(virStorageVolDef) def = NULL; + virStorageVolDefPtr ret = NULL; virStorageVolOptionsPtr options; char *type = NULL; char *allocation = NULL; @@ -1187,132 +1188,132 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (options == NULL) return NULL; - if (VIR_ALLOC(ret) < 0) + if (VIR_ALLOC(def) < 0) return NULL; - ret->target.type = VIR_STORAGE_TYPE_FILE; + def->target.type = VIR_STORAGE_TYPE_FILE; - ret->name = virXPathString("string(./name)", ctxt); - if (ret->name == NULL) { + def->name = virXPathString("string(./name)", ctxt); + if (def->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing volume name element")); - goto error; + goto cleanup; } /* Normally generated by pool refresh, but useful for unit tests */ - ret->key = virXPathString("string(./key)", ctxt); + def->key = virXPathString("string(./key)", ctxt); /* Technically overridden by pool refresh, but useful for unit tests */ type = virXPathString("string(./@type)", ctxt); if (type) { - if ((ret->type = virStorageVolTypeFromString(type)) < 0) { + if ((def->type = virStorageVolTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume type '%s'"), type); - goto error; + goto cleanup; } } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) - goto error; + if (VIR_ALLOC(def->target.backingStore) < 0) + goto cleanup; - ret->target.backingStore->type = VIR_STORAGE_TYPE_FILE; + def->target.backingStore->type = VIR_STORAGE_TYPE_FILE; - ret->target.backingStore->path = backingStore; + def->target.backingStore->path = backingStore; backingStore = NULL; if (options->formatFromString) { char *format = virXPathString("string(./backingStore/format/@type)", ctxt); if (format == NULL) - ret->target.backingStore->format = options->defaultFormat; + def->target.backingStore->format = options->defaultFormat; else - ret->target.backingStore->format = (options->formatFromString)(format); + def->target.backingStore->format = (options->formatFromString)(format); - if (ret->target.backingStore->format < 0) { + if (def->target.backingStore->format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); VIR_FREE(format); - goto error; + goto cleanup; } VIR_FREE(format); } - if (VIR_ALLOC(ret->target.backingStore->perms) < 0) - goto error; - if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms, + if (VIR_ALLOC(def->target.backingStore->perms) < 0) + goto cleanup; + if (virStorageDefParsePerms(ctxt, def->target.backingStore->perms, "./backingStore/permissions") < 0) - goto error; + goto cleanup; } capacity = virXPathString("string(./capacity)", ctxt); unit = virXPathString("string(./capacity/@unit)", ctxt); if (capacity) { - if (virStorageSize(unit, capacity, &ret->target.capacity) < 0) - goto error; + if (virStorageSize(unit, capacity, &def->target.capacity) < 0) + goto cleanup; } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && - virStorageSourceHasBacking(&ret->target))) { + virStorageSourceHasBacking(&def->target))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); - goto error; + goto cleanup; } VIR_FREE(unit); allocation = virXPathString("string(./allocation)", ctxt); if (allocation) { unit = virXPathString("string(./allocation/@unit)", ctxt); - if (virStorageSize(unit, allocation, &ret->target.allocation) < 0) - goto error; - ret->target.has_allocation = true; + if (virStorageSize(unit, allocation, &def->target.allocation) < 0) + goto cleanup; + def->target.has_allocation = true; } else { - ret->target.allocation = ret->target.capacity; + def->target.allocation = def->target.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_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); VIR_FREE(format); - goto error; + goto cleanup; } VIR_FREE(format); } - if (VIR_ALLOC(ret->target.perms) < 0) - goto error; - if (virStorageDefParsePerms(ctxt, ret->target.perms, + if (VIR_ALLOC(def->target.perms) < 0) + goto cleanup; + if (virStorageDefParsePerms(ctxt, def->target.perms, "./target/permissions") < 0) - goto error; + goto cleanup; node = virXPathNode("./target/encryption", ctxt); if (node != NULL) { - ret->target.encryption = virStorageEncryptionParseNode(node, ctxt); - if (ret->target.encryption == NULL) - goto error; + def->target.encryption = virStorageEncryptionParseNode(node, ctxt); + if (def->target.encryption == NULL) + goto cleanup; } - ret->target.compat = virXPathString("string(./target/compat)", ctxt); - if (virStorageFileCheckCompat(ret->target.compat) < 0) - goto error; + def->target.compat = virXPathString("string(./target/compat)", ctxt); + if (virStorageFileCheckCompat(def->target.compat) < 0) + goto cleanup; if (virXPathNode("./target/nocow", ctxt)) - ret->target.nocow = true; + def->target.nocow = true; if (virXPathNode("./target/features", ctxt)) { if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0) - goto error; + goto cleanup; - if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0) - goto error; + if (!def->target.compat && VIR_STRDUP(def->target.compat, "1.1") < 0) + goto cleanup; - if (!(ret->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) - goto error; + if (!(def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) + goto cleanup; for (i = 0; i < n; i++) { int f = virStorageFileFeatureTypeFromString((const char*)nodes[i]->name); @@ -1320,13 +1321,15 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (f < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported feature %s"), (const char*)nodes[i]->name); - goto error; + goto cleanup; } - ignore_value(virBitmapSetBit(ret->target.features, f)); + ignore_value(virBitmapSetBit(def->target.features, f)); } VIR_FREE(nodes); } + VIR_STEAL_PTR(ret, def); + cleanup: VIR_FREE(nodes); VIR_FREE(allocation); @@ -1335,11 +1338,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(type); VIR_FREE(backingStore); return ret; - - error: - virStorageVolDefFree(ret); - ret = NULL;