Re: [PATCH] Add "is_power_of_2" checking to log2.h.

On Wed, 31 Jan 2007, Nick Piggin wrote:

Robert P. J. Day wrote:
On Tue, 30 Jan 2007, Nick Piggin wrote:


Robert P. J. Day wrote:

Add the inline function "is_power_of_2()" to log2.h, where the value
zero is *not* considered to be a power of two.

Signed-off-by: Robert P. J. Day <rpjday@xxxxxxxxxxxxxx>

/*
+ * Determine whether some value is a power of two, where zero is
+ * *not* considered a power of two.
+ */

Why the qualifier? Zero *is* not a power of 2, is it?


no, but it bears repeating since some developers might think it *is*.
if you peruse the current kernel code, you'll find some tests of the
simpler form:

((n & (n - 1)) == 0))

which is clearly testing for "power of twoness" but which will
return true for a value of zero. that's wrong, and it's why it's
emphasized in the comment.

I would have thought you'd comment the broken ones, but that's just
me.

good point, so let's just sum up here. (man, it's hard to believe
that something this simple could drag on so long. i feel like i'm
discussing free device driver development or something. :-)

the new is_power_of_2() macro is defined as:

(n != 0 && ((n & (n - 1)) == 0))

which (correctly, IMHO) does *not* identify zero as a power of two.
if someone truly wants *that* test, they can write it themselves:

if (x == 0 || is_power_of_2(x))

this means that, if someone wants to start rewriting those tests in
the source tree, every time they run across an apparent "power of two"
test of the simpler form:

(n & (n - 1))

they have to ask themselves, "ok, did this coder mean to include zero
or not?" in some cases, it's probably not going to be obvious.
(maybe the maintainers could do a quick check themselves and make the
substitution 'cuz, once the kernel janitors get ahold of this, you
never know what hilarity will ensue. :-)

as far as the patch itself i submitted is concerned, the *only*
place that changed the existing semantics was here:

=================================================
--- a/arch/ppc/syslib/ppc85xx_rio.c
+++ b/arch/ppc/syslib/ppc85xx_rio.c
@@ -59,8 +59,6 @@
#define DBELL_TID(x) (*(u8 *)(x + DOORBELL_TID_OFFSET))
#define DBELL_INF(x) (*(u16 *)(x + DOORBELL_INFO_OFFSET))

-#define is_power_of_2(x) (((x) & ((x) - 1)) == 0)
-
struct rio_atmu_regs {
u32 rowtar;
u32 pad1;
=================================================

so if the powerpc people are ok with that, then the patch itself
should be fine, and it's only the upcoming substitutions in the source
tree that will have to be checked carefully, one by one.

rday

--

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================
-
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/

Relevant Pages

  • Re: Fw: signed kernel modules?
    ... I did a module signing patch some years ago. ... I got scared of its power. ... I guess they'd be happy to have a 'feature' like this in the kernel. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: Question about "Another look at reflections" article.
    ... Correction, it must be moving past a point, not just moving laterally ... average power anywhere on a wire containing a pure standing wave. ... there is zero net energy flow anywhere on a pure standing ...
    (rec.radio.amateur.antenna)
  • Re: Power Analysis and Multiple Regression and Indifference
    ... is statistically significant yet so close to zero as to be indifferent ... effect exists, and is statistically significant, there is little effort ... You can sample from houses with and without a bad Feature, ... power analysis in a usual way for detecting a difference from *0*. ...
    (sci.stat.math)
  • Re: CreateFile() and FILE_FLAG_WRITE_THROUGH
    ... On restart, the file testfile.txt exists but is zero bytes long because although ... So our so-called safe write to disk, is in this situation completely useless! ... We can then happily cut the power, confident that the data will have been ...
    (microsoft.public.win32.programmer.kernel)
  • Re: CreateFile() and FILE_FLAG_WRITE_THROUGH
    ... On restart, the file testfile.txt exists but is zero bytes long because ... So our so-called safe write to disk, ... We can then happily cut the power, confident that the data will have been ...
    (microsoft.public.win32.programmer.kernel)