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.