
[...]
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