[PATCH] do not call _dbus_warn_check_failed on checks

Timo Hoenig thoenig at suse.de
Mon Nov 13 12:18:44 PST 2006


On Mon, 2006-11-13 at 13:01 -0500, Havoc Pennington wrote:

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

You find a patch for this attached to this mail.

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

Agreed.

It would have been nice if there had been a note in the release notes or
at least on-list.  Following yet another bugzilla to track important
changes like should not be required.  In that particular case there is
not even an explanatory comment in the ChangeLog.

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

I agree with all of that.  Just without any notice I was very puzzled
about the crucial change of behavior.

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

I agree on that, too.  That's the way to go.

> Havoc

Thanks,

   Timo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-fix-comment-for-disable-dbus-check-thoenig-01.patch
Type: text/x-patch
Size: 1088 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20061113/a4209fd5/dbus-fix-comment-for-disable-dbus-check-thoenig-01.bin


More information about the dbus mailing list