On 08/28/2013 08:22 AM, 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;
}
Yes it does. I amended the patch, added a line in the commit message
mentioning this leak is fixed as a side effect and pushed.
Thanks for the reviews,
Martin