[PATCH] do not call _dbus_warn_check_failed on checks
hp at redhat.com
Mon Nov 13 10:01:46 PST 2006
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
> 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.
More information about the dbus