Messages in this thread | | | Date | Mon, 22 Aug 2016 10:53:07 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v3 4/5] bug: Provide toggle for BUG on data corruption |
| |
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
| |