Re: [Qemu-devel] [PATCH] Fix compiler warning (always return a value)

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> On Fri, Oct 28, 2011 at 12:07 PM, Markus Armbruster <address@hidden> wrote:
>> Stefan Hajnoczi <address@hidden> writes:
>>
>>> On Fri, Oct 28, 2011 at 9:59 AM, Markus Armbruster <address@hidden> wrote:
>>>> Stefan Hajnoczi <address@hidden> writes:
>>>>
>>>>> On Fri, Oct 28, 2011 at 8:40 AM, Markus Armbruster <address@hidden> wrote:
>>>>>> Stefan Hajnoczi <address@hidden> writes:
[...]
>>>>>>> I would replace assert(3) with abort(3).  If this ever happens the
>>>>>>> program is broken - returning 0 instead of an undefined value doesn't
>>>>>>> help.
>>>>>>
>>>>>> Why is it useful to make failed assertions stop the program regardless
>>>>>> of NDEBUG only when the assertion happens to be "can't reach"?
>>>>>
>>>>> My point is that it should not be an assertion.  The program has a
>>>>> control flow path which should never be taken.  In any "safe"
>>>>> programming environment the program will terminate with a diagnostic.
>>>>
>>>> In the not-so-safe C programming environment, we can disable that safety
>>>> check by defining NDEBUG.
>>>>
>>>> Disabling safety checks is almost always a bad idea.
>>>
>>> There's a difference in a safety check that slows down the system and
>>> a failure condition where the program must terminate.
>>>
>>> assert(3) is for safety checks that can be disabled because they may
>>> slow down the system.
>>>
>>> I've been saying that assert(3) isn't appropriate here because the
>>> program can't continue and there's no check we can skip here.
>>
>> a. Program can't continue: same for pretty much any assertion anywhere.
>>
>> b. There's no code we can skip here: calling abort() is code.
>>
>> What I've been saying is
>>
>> 1. Attempting to distinguish between safety checks that are safe to skip
>> and failure conditions that aren't is a fool's errand.  If a check is
>> safe to skip it's not a safety check by definition.  It's debugging
>> code, perhaps.
>>
>> 2. Optionally disabling "expensive" safety checks should be done based
>> on measurements, if at all.  Arbitrarily declaring all "can't reach"
>> checks "inexpensive" and all other assertions "expensive" isn't
>> measuring, it's guesswork.
>
> I'm tempted to continue the thread but at the end of the day we just
> need to make the function compile with -NDEBUG.  Using abort(3) or

Do we?

Anyone crazy^Wadventurous enough to compile with -DNDEBUG should be
capable of configure --disable-werror.

> qemu_abort() would be the smallest and IMO sanest change, but if it's
> something else that's fine.