Re: [PATCH] VFS: Sanity check mount flags passed to change_mnt_propagation()

[Linus Torvalds]

On Thu, Aug 26, 2010 at 6:14 PM, Matthew Wilcox <matthew@xxxxxx> wrote:
>
> Hrm.  I think we can do this a bit more pithily.

Well, perhaps, but please NOT the way you suggest.

>
>        /* Only one propagation flag should be set, and no others */
>        if (hweight32(type) != 1

Guys, stop with "teh crazy".

What the f*ck kind of expression is that? We don't do this kind of
crap. Even if it's possible that the compiler might be able to
optimize it, it's just crazy to call "hweight32()" for something like
this.

Please think about what you're really after for a second, and realize
that "one bit set" is just another way of saying "power of two". And
then sit back, relax, and realize that there are way better ways to
say "is this is a power of two" than counting bits, for chrissake!

Just a simple "is_power_of_2()" would work. But for anybody who cares
about how it works, here's how to see if there is just a single bit
set:

   n & (n-1)

and then zero is a special case. _Why_ it works is left as an exercise
for the reader, because it's an interesting trick that becomes obvious
once you start thinking about how borrowing works in binary
arithmetic, and what that "subtract one" does for the borrow case for
a power-of-two value vs a non-power-of-two. You can also think about
why zero is a special case, and why you might sometimes consider it an
acceptable value (it's basically the overflow case for shifting bits).

> Too clever?

Not clever at all, I'm afraid.

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/