Re: [libvirt] [PATCH 07/15] storage: Use VIR_AUTOFREE for storage backends
[...]
> 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(a)redhat.com>