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

Andrew Jones
2014-06-12T11:07:30Z

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.

drew