[libvirt] [PATCH] virsh: Fix debugging

[Martin Kletzander]

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