Re: [libvirt] [PATCH] virsh: Fix debugging

On 28.08.2013 08:22, Ján Tomko wrote: > On 08/28/2013 03:39 AM, Eric Blake wrote: >> On 08/27/2013 06:27 AM, Martin Kletzander wrote: >>> diff --git a/tools/virsh.c b/tools/virsh.c >>> index ac77156..34f5c4a 100644 >>> --- a/tools/virsh.c >>> +++ b/tools/virsh.c >>> @@ -2321,10 +2321,9 @@ vshInitDebug(vshControl *ctl) >>> debugEnv = getenv("VIRSH_LOG_FILE"); >>> if (debugEnv && *debugEnv) { >>> ctl->logfile = vshStrdup(ctl, debugEnv); >>> + vshOpenLogFile(ctl); >>> } >>> } >>> - >>> - vshOpenLogFile(ctl); >>> } >>> >>> /* >>> @@ -3044,7 +3043,9 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) >>> ctl->readonly = true; >>> break; >>> case 'l': >>> + vshCloseLogFile(ctl); >>> ctl->logfile = vshStrdup(ctl, optarg); >>> + vshOpenLogFile(ctl); >> >> Note that there's another leak here (pre-existing): If I call: >> virsh -l file1 -l file2, we leak "file1" by reassigning a malloc'd >> string without first freeing the old version. >> > > Actually, it gets freed in vshCloseLogFile (and set to NULL, just to be double > sure): > > if (ctl->logfile) { > VIR_FREE(ctl->logfile); > ctl->logfile = NULL; BTW this makes no sense, since VIR_FREE() sets every passed pointer to NULL. so this assignment is just statement without any effect.