Eric Blake wrote: > On 05/21/2014 08:02 AM, Roman Bogorodskiy wrote: > > >>>> So, what you propose is checking explicitly for NULL in > >>>> virObjectUnlock before we do virObjectIsClass(), and if the passed > >>>> argument is NULL indeed, just return, without logging anything about > >>>> that? > >>> > >>> Yes, since we have other virObject code that special cases NULL (for > >>> example, virObjectUnref). > >> > >> IMHO passing NULL into the locking APIs is a coding error we shouldn't > >> try to paper over by ignoring. > >> > >> Ultimately I think the real flaw is the way we obtain the virDomainPtr > >> pointers in the first place. ie the virDomainObjListLookup functions > >> don't acquire a reference on the object they return. So the caller has > >> to worry about the object being released behind their back, hence all > >> our logic which has to set 'vm = NULL' in various places where it might > >> have been deleted. > >> > >> IOW, I'd much rather we looked at changing our design here so that we > >> didn't have so much NULL vm pointers in the first place. > > > > Eric, could you please comment on that? > > I trust Daniel's judgment here, since he wrote virObjectPtr and > virObjectUnlock. Cleaning up the DomainObjListLookup function to grab a > reference will have a big ripple effect, so it probably doesn't need to > be done now (that is, you don't necessarily have to be the one doing the > cleanup he proposes); but I guess that means for the present that we are > still stuck with the current design pattern of checking for NULL ourself > before calling virObjectUnlock. OK, then I'll push the current bhyve patch that was already ACKed by Daniel and leave virObjectUnlock related cleanup for now; thanks. Roman Bogorodskiy
Attachment:
pgpgzvnQ81tBW.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list