[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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
==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.

Hu Tao

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]