Re: [libvirt] [PATCH] fix memory leak in virCopyLastError
On Fri, Sep 14, 2012 at 03:10:13PM +0800, Daniel Veillard wrote:
On Fri, Sep 14, 2012 at 02:24:15PM +0800, Hu Tao wrote:
> memset before virResetError will cause memory leak.
>
> virResetError and virCopyError, which calls virResetError, will do
> memset properly, so we don't have to worry about it here.
Disagree, it's a public API, we can't justify behaviour just
on how it is used internally.
NACK, at least the explanation need to be fixed
What is the scenario for the leak ?
The leaked memory was allocated at qemu_monitor.c:636. One of the leak
reported by valgrind is:
==12636== 40 bytes in 1 blocks are definitely lost in loss record 302 of
620
==12636== at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==12636== by 0x306B27FC01: strdup (in /lib64/libc-2.13.so)
==12636== by 0x4EA5669: virCopyError (virterror.c:182)
==12636== by 0x4EA573C: virCopyLastError (virterror.c:282)
==12636== by 0x110CFEA9: qemuMonitorIO (qemu_monitor.c:636)
==12636== by 0x4E83950: virEventPollRunOnce (event_poll.c:485)
==12636== by 0x4E82004: virEventRunDefaultImpl (event.c:247)
==12636== by 0x4F822BC: virNetServerRun (virnetserver.c:751)
==12636== by 0x40C433: main (libvirtd.c:1338)
The scenario is: If we deep-copy a virError, by virCopyLastError, into
another virError object which is previously deep-copied into, then we
have no chance to free previously allocated memory, because the memset
in virCopyLastError loses any pointers to them.
--
Thanks,
Hu Tao