Re: Abortions

[Ben Pfaff]

John Darrington <address@hidden> writes:

> It's come to my attention, that there are a lot of calls to abort()
> dotted around the code.  The only place that should call abort() IMHO
> is request_bug_report_and_abort() in message.c
>
> All the others should be changed to assert(false) --- which raises
> another issue: there should be no code which #includes <assert.h>
> because we have our own custom assert in message.h

I don't like the idea of trying to replace the standard assert
macro with our own implementation, even I know we've been doing
so for quite a while.  First, it's surprising to type "assert"
and not get the standard assert macro.  Second, the standard
strongly discourages trying to define reserved names such as
assert.  Third, code that looks correct (that #includes
<assert.h>) isn't.  Fourth, message.h doesn't honor NDEBUG
(although that's of course fixable).

Let me propose a change.  Instead of introducing our own assert
macro, let's use the standard one.  The standard guarantees that
an assertion failure prints a message on stderr that includes all
the info we want and then calls abort().  So we can just catch
SIGABRT and write out a message like "You've found a bug in PSPP,
please report the assertion failure message above, if present,
plus the other info below to bug-pspp".  This would be cleaner,
in my opinion.

assert(false) is a separate issue.  It doesn't do exactly what
abort() does--when you compile with NDEBUG, it doesn't do
anything.  That means that GCC will start warning if you were
using it to mark code that should never be reached, such as a
"default" case in a switch statement, and that now looks like it
fails to initialize some variable that all the other cases do
initialize.  message.h doesn't honor NDEBUG, so this behavior
probably doesn't show up much in PSPP.

I'd actually prefer to do for assert(false)/abort() what I've
done in some other projects: add a new macro NOT_REACHED() that
expands to
        assert (false), abort ()
This has the desired effect whether NDEBUG is enabled or not.

What do you think?
-- 
"MONO - Monochrome Emulation
 This field is used to store your favorite bit."
--FreeVGA Attribute Controller Reference