[libcamera-devel] [PATCH 02/11] libcamera: log: Add an ASSERT macro
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jan 7 12:20:01 CET 2019
Hi Jacopo,
On Monday, 7 January 2019 12:25:13 EET Jacopo Mondi wrote:
> On Sun, Jan 06, 2019 at 04:33:19AM +0200, Laurent Pinchart wrote:
> > The ASSERT() macro is similar to the assert() macro defined by the C
> > standard, but uses the libcamera logging infrastructure.
>
> ../src/libcamera/include/log.h:42:45: error: expected primary-expression
> before ‘{’ token #define ASSERT(condition) static_cast<void>({ \
> ^
> ../src/libcamera/media_device.cpp:267:2: note: in expansion of macro
> ‘ASSERT’ ASSERT(valid_);
> ^~~~~~
>
> Am I the only one?
No, after fixing the #if(n)def NDEBUG issue Niklas pointed out, I received the
same error. I've fixed it by adding an extra () around the {} statement.
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >
> > src/libcamera/include/log.h | 9 +++++++++
> > src/libcamera/log.cpp | 16 ++++++++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h
> > index 03842be02d0e..774916f04274 100644
> > --- a/src/libcamera/include/log.h
> > +++ b/src/libcamera/include/log.h
> >
> > @@ -36,6 +36,15 @@ private:
> > #define LOG(severity) LogMessage(__FILE__, __LINE__,
> > Log##severity).stream()>
> > +#ifdef NDEBUG
> > +#define ASSERT(condition) static_cast<void>({ \
>
> Why do you use a cast to void? Isn't this better wrapped in a
> canonical "do { } while(0)" ?
I don't know what the pros and cons of each option are, they're both used
widely.
> > + if (condition) \
>
> I feel we should trigger this if the condition is not satisfied, not
> the other way around, am I wrong?
> http://www.cplusplus.com/reference/cassert/assert/
I must have been really tired when I wrote this... Will fix.
> > + LOG(Fatal) << "assertion \"" #condition "\" failed"; \
> > +})
> > +#else
> > +#define ASSERT(condition) static_cast<void>(false && (condition))
> > +#endif
> > +
> > } /* namespace libcamera */
> >
> > #endif /* __LIBCAMERA_LOG_H__ */
> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> > index a5823c64eaa6..4165cbd654fc 100644
> > --- a/src/libcamera/log.cpp
> > +++ b/src/libcamera/log.cpp
> > @@ -48,6 +48,22 @@ namespace libcamera {
> >
> > * terminates immediately after printing the message.
> > */
> >
> > +/**
> > + * \def ASSERT(condition)
> > + * \brief Abort program execution if assertion is failed
>
> s/is failed/fails ?
Fixed.
> > + *
> > + * If \a condition is false, ASSERT() logs an error message with the
> > Fatal log + * level and aborts program execution.
>
> Ah, ok, seems like it should then be "if (!(condition))" according to
> documentation too.
At least the documentation is right :-)
> > + *
> > + * If the macro NDEBUG is defined before including log.h, ASSERT()
> > generates no + * code.
> > + *
> > + * Using conditions that have side effects with ASSERT() is not
> > recommended, as + * these effects would depend on whether NDEBUG is
> > defined or not. Similarly, + * ASSERT() should not be used to check for
> > errors that can occur under normal + * conditions as those checks would
> > then be removed when compiling with NDEBUG.
> > + */
> > +
> > static const char *log_severity_name(LogSeverity severity)
> > {
> > static const char * const names[] = {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list