Hello,

[added some more context again]

On Tue, Feb 03, 2015 at 03:14:43PM +0000, David Howells wrote:

quoted

quoted

-     *p  |= mask;
+     if ((*p & mask) == 0)
+             *p  |= mask;

Care to fix the double space here while touching the code?

I think the more natural check here is:

        if ((~*p & mask) != 0)
                *p |= mask;

Might be a matter of taste, but this check is equivalent to

	*p != (*p | mask)

which is what you really want to test for.

I would argue that this is less clear as to what's going on.

OK, I admit that this equivalence is not obvious. Then maybe let the
compiler find the equivalence and do:

-	*p  |= mask;
+	if (*p != (*p | mask))
+		p |= mask;

?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |