Re: [libvirt] [PATCH] daemon: plug a memory leak

On 12/13/2010 08:59 AM, Laine Stump wrote: > On 12/10/2010 07:29 PM, Eric Blake wrote: >> * daemon/libvirtd.c (qemudFreeClient): Avoid a leak. >> (qemudDispatchServer): Avoid null dereference. >> --- >> >> I keep finding more of these. >> > > ACK. Thanks; pushed. > > Have you looked into some of the other stuff in the qemud_client struct? > For example, nothing inside #ifdef HAVE_SASL gets freed during > qemudFreeClient(). Quickly looking at the use of those items, it appears > it might be possible to be freeing up the client struct when one of > those is non-NULL, but I didn't read very carefully... I saw that in code inspection as well, but have not yet seen it show up in a valgrind run (that's not to say it is leak-free, so much as I haven't yet exercised libvirt under the right scenarios to trigger the use of sasl in the first place). So there may indeed be a leak, and I don't know how hard it would be to chase it down. I'd almost rather see danpb's rewrite to move all SASL handling into new server/client wrappers (right now, it's duplicated across daemon and remote handling), then double check that the refactored code correctly cleans up SASL data.