On Fri, Sep 14, 2012 at 03:34:29PM +0800, Hu Tao wrote: > 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. So we have a problem here, we use virCopyLastError() t copy errors internally in location where an error might have been allocated, and that's also a public entry point where we can't trust the user provided data. We can't change the API behaviour so virCopyLastError() has to keep the memset, but we should do an private function virCopyLastErrorinternal() which would deallocate existing data in @to before calling virCopyError() Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list