On Mon, Aug 22, 2016 at 03:15:35PM +0200, Arnd Bergmann wrote:
> 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; \
> })
That does look better, now that you mention it. Kees, any objections?
Thanx, Paul
