[libvirt] [PATCH 7/7] util: Add safety net of checks to ensure valid object

[John Ferlan]

[...]


> 
> 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