[PATCH] do not call _dbus_warn_check_failed on checks

[Havoc Pennington]

Timo Hoenig wrote:
> 
> Interesting.  I would have expected to see such a grave change in
> behavior being brought up on-list.
> 
> So basically we're not having a bunch of checks which are not checks but
> assertions (as long as DBUS_FATAL_WARNINGS is not explicitly set to
> '0').
> 
> Havoc, as you own the commit I am questioning, could you please comment?

This is an intentional change. If we wanted to revert it, then the right 
fix is to toggle the fatalness default in the code for 
warn_check_failed, rather than your patch (though the comment fix in 
your patch would be good to get in).

The change was discussed on several bugzilla bugs at least. Several 
people were unhappy that "behavior was undefined" after check failure, 
wanted guarantees about what was returned and/or internally-consistent 
state post-check-failure, etc. This is not and has never been the 
intent; once the check fails, behavior is undefined. A build of libdbus 
with no checks at all is very legitimate (and will have a bunch of 
segv's where the checks were before).

I think fatal checks gives people the right idea about what the checks 
mean, namely, a bug in their program. Before people seemed to have the 
idea that libdbus worked like system calls - that passing in junk would 
get you EINVAL. I got a little sick of people trying to check the 
behavior of the checks in test suites and otherwise relying on the checks.

The checks are not in the ABI; if a condition that is checked is true, 
then you can't call the function.

I admit fatal checks is a little user-unfriendly. If the programmer 
conscientiously tried to fix all their check failures, but left a bug 
in, then sometimes an app could recover and continue without 
user-visible results. Fatal checks makes that impossible. On other other 
hand, fatal checks means that bug-buddy and equivalents will kick in in 
that case and get the issue reported.

Due to user impact, distributions might want to put 
DBUS_FATAL_WARNINGS=0 in production releases, but I'd encourage them to 
leave the default for test releases and get those crashes fixed.

Havoc