Re: [PATCH v5 09/19] libcflat: add abort() and assert()

Paolo Bonzini
2014-06-12T11:09:38Z

Il 12/06/2014 13:07, Andrew Jones ha scritto:
> On Thu, Jun 12, 2014 at 12:48:16PM +0200, Paolo Bonzini wrote:
>> Il 11/06/2014 16:01, Andrew Jones ha scritto:
>>> The test framework may have external dependencies. assert() provides
>>> the ability to abort when those dependencies aren't met. However,
>>> assert() should only be used for unlikely conditions. We can provide
>>> more informative messages with printf() for the more likely problems.
>>>
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>> ---
>>> lib/libcflat.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>>> index 024c834630d63..8886885766ddc 100644
>>> --- a/lib/libcflat.h
>>> +++ b/lib/libcflat.h
>>> @@ -70,4 +70,13 @@ extern long atol(const char *ptr);
>>>
>>> void report(const char *msg_fmt, bool pass, ...);
>>> int report_summary(void);
>>> +
>>> +#define abort() exit(64) /* 129 exit status from qemu */
>>
>> This can be confused with a SIGHUP. We can change it to exit(44) aka 99
>> exit status.
>>
>> Also, please make it a function rather than a macro.
>
> OK
>
>>
>>> +#define assert(cond) \
>>> +do { \
>>> + if (!(cond)) \
>>> + printf("%s:%d: assert failed\n", __FILE__, __LINE__), \
>>> + abort(); \
>>> +} while (0)
>>
>> Here you could also put the printf/abort into a function; as you prefer.
>>
>
> Might as well. If I'm moving abort() into a general lib function, then
> I'll need to find a home for it. I can use the same home for assert().
> The best existing home is probably lib/report.c, or I should create a
> new .c file.

A new lib/abort.c file sounds better.

Paolo