On 07/25/2017 07:21 AM, Pavel Hrdina wrote:
> On Fri, Jul 14, 2017 at 10:04:39AM -0400, John Ferlan wrote:
>> 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.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/conf/virsecretobj.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
>> index e3bcbe5..bedcdbd 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)
>> @@ -369,8 +368,9 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
>> virSecretDefFree(objdef);
>> obj->def = newdef;
>> } else {
>> +
>> /* No existing secret with same UUID,
>> - * try look for matching usage instead */
>> + * try to look for matching usage instead */
>> if ((obj = virSecretObjListFindByUsageLocked(secrets,
>> newdef->usage_type,
>> newdef->usage_id))) {
>> @@ -381,7 +381,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
>> _("a secret with UUID %s already defined for "
>> "use with %s"),
>> uuidstr, newdef->usage_id);
>> - goto cleanup;
>> + goto error;
>> }
>>
>> /* Generate the possible configFile and base64File strings
>> @@ -395,7 +395,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
>> goto cleanup;
>>
>> if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)
>> - goto cleanup;
>> + goto error;
>>
>> obj->def = newdef;
>> VIR_STEAL_PTR(obj->configFile, configFile);
>> @@ -403,15 +403,15 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
>> virObjectRef(obj);
>> }
>>
>> - ret = obj;
>> - obj = NULL;
>> -
>> cleanup:
>> - virSecretObjEndAPI(&obj);
>> VIR_FREE(configFile);
>> VIR_FREE(base64File);
>> virObjectUnlock(secrets);
>> - return ret;
>> + return obj;
>> +
>> + error:
>> + virSecretObjEndAPI(&obj);
>> + goto cleanup;
>
> I don't see what's wrong with the current code, it's commonly used
> pattern to steal the pointer. The extra error label would make sense
> if the error path would be more complex, but in this case it doesn't
> add any value.
>
> Pavel
>
Fine - I'll drop this one then.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
