Clean up OSL_ASSERT, DBG_ASSERT, etc.

[Michael Stahl]

On 16.01.2015 15:14, Stephan Bergmann wrote:
> On 01/16/2015 01:26 PM, Ashod Nakashian wrote:

>> code, modified code, or by a new external input,) which is vital for
> 
> Not sure I understand you here, but note that input data in itself 
> should never be the reason for a failed precondition.  A failed 
> precondition is always an indicator of a programming error.

right, best not to confuse validation of untrusted input (where debug
logging / SAL_INFO() is appropriate and checks must never be removed in
release builds) and bugs in the program code (where assert() is
appropriate).

> [...]
>> A quick grep returns the following assertion functions (those
>> containing case-insensitive "assert"): assert, DBG_ASSERT, OSL_ASSERT,
>> DBG_BF_ASSERT. This is excluding CPPUNIT_ASSERT and assertions in
>> 3rdparty libraries (which are out of scope).
> 
> History shows that moving from the legacy zoo of ill-specified, 
> inconsistently-used functionality to a supposedly sane set is by no 
> means trivial and is taking a long long time to accomplish.

indeed, the current problem is not that we don't know what the desirable
assertion/logging facilities should be, but consistently deploying them
throughout the code.

> It is the inconsistent usage of the legacy functionality that makes it 
> hard to properly map to one of the three to four advocated mechanisms du 
> jour (static_assert, assert, SAL_LOG_IF, SAL_WARN_IF), and that 
> apparently makes easy hackers shy away from it.

yes, and i'd go further to say that having easy-hackers convert existing
legacy assertions to assert() in particular is a bad idea: if done at
scale it could tie up a lot of developers investigating and fixing up
over time a large number of cases when the legacy assertion was actually
bogus or just intended as a "warning" about something "potentially
suspicious" that then manifest as crashes in debug builds.  the average
easy-hacker can't be expected to determine when an assert() is
appropriate and when a SAL_WARN(), and in my experience doing that is
often hard in code with which i'm not familiar.  that's also the reason
why i don't do too many conversions to assert() at a time, to limit the
time i'll spend fixing the cases when i guessed wrong; better to let
things soak for a week or two and then continue.

>> 3) FailBreak: Calls FailLog then breaks into the debugger (typically
>> by issuing 'int 3' on x86/x64). (This might not be useful on
>> non-Windows platforms.)
> 
> My understanding is that Windows alrady offers this "break into the 
> debugger" for plain abort?

yes it does, it pops up a dialog box. sometimes the button to start the
debugger doesn't work, but it always works to start visual studio
manually and attach to the process (which is blocking on the dialog box).