On 02/21/2012 02:08 AM, Eric Blake wrote:
[side note - top-posting on technical lists makes it a bit harder to follow the conversation - I've reformatted my reply]
Hello Eric,
I wouldn't be surprised if there is still a lurking bug here; after all, my commit 15a280bb was an attempt to solve a valgrind memory leak, which was then in turn reworked by Jim in commits c05ec920 and fcdfa31f. But we need to diagnose and patch the real problem, if this is the case. As you said, it's impossible to fix this leak by my change, and from codes point of view, the failure path hasn't any memory leak. I suspect valgrind gave a incorrect report, and I will check it later. At this point, I've re-read fcdfa31f, and don't see any leak of ident. I suspect that what really happened is that you ran valgrind on 0.9.10,
On 02/20/2012 09:06 AM, Alex Jia wrote: prior to my commit 15a280b, where there really was a memleak on ident; but then wrote your patch after upgrading to latest rather than re-checking valgrind to see if libvirt.git had fixed the bug in the meantime. Since my commit 15a280b didn't mention plugging a memory leak, nor that I had found the regression in question by using valgrind, I can see why I might have confused you. So not only did I cause problems by introducing the leak prior to 0.9.10, I compounded it in my patch to fix the leak. Sorry for the mess I caused :) Thanks, it's okay for me, I should see patches more careful next time :)
Regards, Alex