Re: [PATCH v3 4/5] bug: Provide toggle for BUG on data corruption

[Arnd Bergmann]

On Wednesday, August 17, 2016 2:42:11 PM CEST Kees Cook wrote:
> +
> +/*
> + * Since detected data corruption should stop operation on the affected
> + * structures, this returns false if the corruption condition is found.
> + */
> +#define CHECK_DATA_CORRUPTION(condition, fmt, ...)                      \
> +       do {                                                             \
> +               if (unlikely(condition)) {                               \
> +                       if (IS_ENABLED(CONFIG_BUG_ON_DATA_CORRUPTION)) { \
> +                               pr_err(fmt, ##__VA_ARGS__);              \
> +                               BUG();                                   \
> +                       } else                                           \
> +                               WARN(1, fmt, ##__VA_ARGS__);             \
> +                       return false;                                    \
> +               }                                                        \
> +       } while (0)
> +

I think the "return false" inside of the macro makes it easy to misread
what is actually going on.

How about making it a macro that returns the condition argument?

#define CHECK_DATA_CORRUPTION(condition, fmt, ...)	\
({	\
	bool _condition = unlikely(condition);	\
	if (_condition) {	\
		...
	}	\
	_condition;	\
})

	Arnd