Re: [git pull] x86 fixes

Pallipadi, Venkatesh
2009-01-12T19:54:52Z

On Mon, Jan 12, 2009 at 11:47:13AM -0800, Linus Torvalds wrote:
>
>
> On Mon, 12 Jan 2009, Pallipadi, Venkatesh wrote:
> > + if (strict_prot ||
> > + (want_flags == _PAGE_CACHE_UC_MINUS &&
> > + flags == _PAGE_CACHE_WB) ||
> > + (want_flags == _PAGE_CACHE_WC &&
> > + flags == _PAGE_CACHE_WB)) {
>
> Please don't write code like this.
>
> Do it as an inline function that returns true/false and has comments on
> what the hell is going on.
>
> If a conditional doesn't fit on one line, it should generally be
> abstracted away into a readable function where the name explains what it
> does conceptually.
>

Yes. The actual patch that is lined up in tip fixes indeed has this as a
macro sharing this code with 2 callers and comment about this
(is_new_memtype_allowed()). I wanted to keep the changes smaller in this
test patch, which is just to root cause this particular crash and ended
up with above code.

Thanks,
Venki