[...]
>> 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@xxxxxxxxxx>
>
