On 07.09.2015 09:23, Martin Kletzander wrote:
On Wed, Sep 02, 2015 at 05:58:18PM +0200, Michal Privoznik wrote:
Let's move some variables from an inside loop to global function declaration header block. It's going to be easier for next patches. At the same time, order the cleanup calls at the function's end so it's easier to track which variables are freed and which not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
I did not follow the discussion around 2/2, but this one does not do any functional change and cleans up the code nicely, so ACK ... [1]
Pushed, thanks.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 340a8e2..d4e500b 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -549,24 +544,28 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) if (details) { vshPrint(ctl, "%-10s %-10s %-10s %s\n", type, device, target, source ? source : "-"); - VIR_FREE(type); - VIR_FREE(device); } else { vshPrint(ctl, "%-10s %s\n", target, source ? source : "-"); }
- VIR_FREE(target); VIR_FREE(source); + VIR_FREE(target); + VIR_FREE(device); + VIR_FREE(type); }
ret = true;
cleanup: + VIR_FREE(source); + VIR_FREE(target); + VIR_FREE(device); + VIR_FREE(type); VIR_FREE(disks); - virDomainFree(dom); - VIR_FREE(xml); - xmlFreeDoc(xmldoc); xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + VIR_FREE(xml); + virDomainFree(dom);
[1] ... even though I don't understand why these are reordered.
It's explained in the commit message. I always felt like cleanup should follow the order laid out by variable definition at function beginning, or in reversed order. If it is random, it's harder to spot a leaking variable. Michal