On Mon, Dec 13, 2010 at 09:50:40AM -0700, Eric Blake wrote: > 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. I've re-written all the SASL auth code from scratch in a shared module between client + server. All the SASL I/O layer is also being pushed down into a shared socket module. This will make it possible to actually unit test >95% of the RPC code without actually running daemons/drivers. Regards, Daniel -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list