[...] > > I really don't think these changes are a positive move. > > If you have code that is passing in something that is not a valid object, > then silently doing nothing in virObjectRef / virObjectIsClass is not > going to make the code any more correct. In fact you're turning something > that could be an immediate crash (and thus easy to diagnose) into > something that could be silent bad behaviour, which is much harder to > diagnose, or cause a crash much further away from the original root bug. > > Consider the longevity of the patch being on list - I cannot remember all the details, although the commit message does help a bit. I do recall looking at the code and thinking incorrect usage could lead down the road of bad things. For Ref/Unref all that has been checked is !obj and we Incr/Decr a location. For Lock/Unlock as described in my 1/7 response class checking is/was added. Adding in object validity for Ref/Unref at least avoids rogue corruption which is really hard to diagnose in favor of leaving an entrail. Even without the additional check, the @obj someone may have thought they were incr/decr the refcnt isn't going to happen. Still, it just seems better in the event that we don't have a real @obj to at least message that in the hopes that someone trips across it. Similarly for Lock/Unlock same thing. IMO: The patches aren't making it harder to diagnose a problem - they're helping and they're not altering the value of some field for a valid @obj address. But if that's not desired, then fine - at least attempt was made and the feedback has been provided. Tks - John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
