On Tuesday 03 November 2009 17:07:32 parawaiter wrote: > On Nov 3, 2009, at 11:44 PM, David N. Lombard wrote: > > On Tue, Nov 03, 2009 at 01:42:48PM -0800, Alexander Shishkin wrote: > >> On Втр, Ноя 03, 2009 at 04:28:09 +0100, Denys Vlasenko wrote: > >>> assert (*buffer == info->buffer); > >>> *buffer = info->buffer; > >> > >> "Never say never". > > > > Ummm, if the order was reversed, one could argue this was an attempt > > to catch a > > silent data corruption (SDC). > > I'd say the order was correct, but you had a freak bug; *buffer should > be info->buffer, but once in a blue moon it isn't. You are trying to > catch it, but haven't been able to figure it out yet. For release > builds you can make sure whatever follows works by ensuring the value > is correct. For debug builds, you are trying to catch it with an > assert. Yeah, it's a crappy workaround that doesn't ensure > correctness, but it could improve release-build robustness an order of > magnitude until you are able to fix the bug. The resulting code looks > ridiculous. > But the more likely explanation for code like this is that the code > transformed from something very different, and it's more or less in > the category of a copy-bug now.
My favorite is where an assert dereferences something to check a field right _before_ the non-assert test that what it's checking isn't null. assert(thingy->field != walrus); if (!thingy) return -EFRUITBASKET; Meaning that code is only buggy when asserts are enabled, and otherwise works fine. Enabling the assert _caused_ a bug. Asserts are horrible. Don't go there. If you want to test for and respond to some condition, do so. If you don't, then don't. But don't _waffle_ about it. Rob -- Latency is more important than throughput. It's that simple. - Linus Torvalds _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
