[libcamera-devel] [PATCH 02/11] libcamera: log: Add an ASSERT macro

[Jacopo Mondi]

Hi Laurent,

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?

> 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)" ?

> +	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/


> +		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 ?

> + *
> + * 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.

Thanks
  j

> + *
> + * 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
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190107/bae1cbab/attachment.sig>