Markus Armbruster writes:
> Lluís Vilanova <vilan...@ac.upc.edu> writes:
>> Markus Armbruster writes:
>>
>>> Lluís Vilanova <vilan...@ac.upc.edu> writes:
>>>> Markus Armbruster writes:
>>>>
>>>>> This overlaps with parts of Lluís's "[RFC][PATCH v6 0/5] utils:
>>>>> Improve and document error reporting". Lluís, feel free to integrate
>>>>> my patches into a respin of your series. You can also respin on top
>>>>> of my series.
>>>>
>>>> Reviewed-by: Lluís Vilanova <vilan...@ac.upc.edu>
>>>>
>>>> I'm happy with this series as a replacement for mine. Two nitpicks:
>>>>
>>>> * The error.h comments point to assert() instead of abort() as a
>>>> replacement for
>>>> error_setg(&error_abort) (while your HACKING says otherwise).
>>
>>> Where?
>>
>> Documentation on error_setg() (last phrase) vs HACKING (also last phrase).
> Got it. error_setg()'s comment:
> Likewise, don't error_setg(&error_abort, ...), use assert().
> This is advice on what to do.
> HACKING:
> Note that &error_fatal is just another way to exit(1), and
> &error_abort is just another way to abort().
> This tries to extend the admonition on exit() and abort() to
> &error_fatal and &error_abort. I'm open for better ways to word it.
> However, I feel this section of HACKING should not go into detail on
> what to do, but leave that to error.h.
Right. I just wanted to say the error.h comment should be:
Likewise, don't error_setg(&error_abort, ...), use abort().
To be consistent with HACKING (which implies the equivalent for error_abort is
abort()). But that's just me thinking that assert will be omitted when NDEBUG is
defined :)
As a side note (just an observation), it seems that NDEBUG is only selectively
(and manually) defined in some files like "tcg/tcg.c", so it's not consistently
affecting files (e.g., it's used as a shorter substitute of "#ifdef
CONFIG_DEBUG_TCG", but it's not acting like a "real" NDEBUG; aka assert will
still work *iff* it's included before defining NDEBUG, which is not clear at
all).
>>>> * HACKING does not explicitly point out that exit(1) is the preferred exit
>>>> code.
>>
>>> Does it need saying? We don't seem to have a weird exit() problem in
>>> new code.
>>
>>> The lower HACKING's signal-to-noise ratio gets, the fewer people will
>>> actually read it attentively.
>>
>> Fine by me.
> Thanks!
Cheers,
Lluis
