Re: [libvirt] [PATCH 4/8] secret: Clean up virSecretObjListAdd processing

Make use of an error: label to handle the failure and need to call virSecretObjEndAPI for the object to set it to NULL for return. Also rather than an if/else processing - have the initial "if" which is just replacing the @newdef into obj->def goto cleanup, thus allowing the remaining code to be extracted from the else and appear to more inline. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virsecretobj.c | 74 ++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index e3bcbe5..1bafd0b 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -333,7 +333,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets, { virSecretObjPtr obj; virSecretDefPtr objdef; - virSecretObjPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; char *configFile = NULL, *base64File = NULL; @@ -354,13 +353,13 @@ virSecretObjListAdd(virSecretObjListPtr secrets, _("a secret with UUID %s is already defined for " "use with %s"), uuidstr, objdef->usage_id); - goto cleanup; + goto error; } if (objdef->isprivate && !newdef->isprivate) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change private flag on existing secret")); - goto cleanup; + goto error; } if (oldDef) @@ -368,50 +367,51 @@ virSecretObjListAdd(virSecretObjListPtr secrets, else virSecretDefFree(objdef); obj->def = newdef; - } else { - /* No existing secret with same UUID, - * try look for matching usage instead */ - if ((obj = virSecretObjListFindByUsageLocked(secrets, - newdef->usage_type, - newdef->usage_id))) { - virObjectLock(obj); - objdef = obj->def; - virUUIDFormat(objdef->uuid, uuidstr); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("a secret with UUID %s already defined for " - "use with %s"), - uuidstr, newdef->usage_id); - goto cleanup; - } + goto cleanup; + } - /* Generate the possible configFile and base64File strings - * using the configDir, uuidstr, and appropriate suffix - */ - if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) || - !(base64File = virFileBuildPath(configDir, uuidstr, ".base64"))) - goto cleanup; + /* No existing secret with same UUID, + * try to look for matching usage instead */ + if ((obj = virSecretObjListFindByUsageLocked(secrets, + newdef->usage_type, + newdef->usage_id))) { + virObjectLock(obj); + objdef = obj->def; + virUUIDFormat(objdef->uuid, uuidstr); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("a secret with UUID %s already defined for " + "use with %s"), + uuidstr, newdef->usage_id); + goto error; + } - if (!(obj = virSecretObjNew())) - goto cleanup; + /* Generate the possible configFile and base64File strings + * using the configDir, uuidstr, and appropriate suffix + */ + if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) || + !(base64File = virFileBuildPath(configDir, uuidstr, ".base64"))) + goto cleanup; - if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0) - goto cleanup; + if (!(obj = virSecretObjNew())) + goto cleanup; - obj->def = newdef; - VIR_STEAL_PTR(obj->configFile, configFile); - VIR_STEAL_PTR(obj->base64File, base64File); - virObjectRef(obj); - } + if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0) + goto error;