On Wed 28 Aug 2013 11:18:43 AM CEST, Michal Privoznik wrote:
> 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.
>
Moreover if ctl->logfile is NULL, it won't do anything, so we can save 3
lines by just doing VIR_FREE without the condition and NULL-reset, yes.
But that's unrelated to the patch ;-)
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list