[libvirt] [PATCH 07/15] storage: Use VIR_AUTOFREE for storage backends

John Ferlan jferlan at redhat.com
Fri Feb 8 15:06:07 UTC 2019


[...]

>> diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c
>> index 82fa4d7a25..6458b0f835 100644
>> --- a/src/storage/storage_backend_iscsi_direct.c
>> +++ b/src/storage/storage_backend_iscsi_direct.c
>> @@ -421,15 +421,14 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi,
>>      }
>>
>>      for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) {
>> -        char *target = NULL;
>> +        VIR_AUTOFREE(char *) target = NULL;
>>
>>          if (VIR_STRDUP(target, tmp_addr->target_name) < 0)
>>              goto cleanup;
>>
>> -        if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) {
>> -            VIR_FREE(target);
>> +        if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0)
>>              goto cleanup;
>> -        }
>> +        target = NULL;
> 
> VIR_APPEND_ELEMENT clears the source, so ^this should not be needed.
> 
> [snip]
> 

Right - just rote habit I guess...

>> -    dev->path = pvname;
>> +    VIR_STEAL_PTR(dev->path, pvname);
> 
> This VIR_STEAL_PTR stuff should come separe as mentioned in previous reviews.
> 

For this one I disagree...  similar to the @vgname, previously we
"failed" through the error: label and returned -1, but with the AUTO
stuff added, we now cannot leave @vgname and @pvname as NULL.

This one needs to stay.

John
> The rest looks fine:
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 




More information about the libvir-list mailing list