public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
From: "Robert P. J. Day" <rpjday@crashcourse.ca>
To: Linus Probert <linus.probert@gmail.com>
Cc: Julia Lawall <julia.lawall@inria.fr>,
	 Kernel Janitors List <kernel-janitors@vger.kernel.org>
Subject: Re: more potential janitor work: simplifying test for power of 2
Date: Thu, 2 Apr 2026 08:40:33 -0400 (EDT)	[thread overview]
Message-ID: <bcd30f78-7cbb-4284-dc18-302b549a00e2@crashcourse.ca> (raw)
In-Reply-To: <DHINFSQNG4ZT.JNZO5ZPRUMW5@gmail.com>

On Thu, 2 Apr 2026, Linus Probert wrote:

> On Thu Apr 2, 2026 at 12:07 PM CEST, Julia Lawall wrote:
> >
> >
> > On Thu, 2 Apr 2026, Linus Probert wrote:
> >
> >> I took a closer look at this. Many of the occurences here are actually
> >> used to check if a binary flag has more then one bit set. This is quite
> >> a common pattern.
> >>
> >> Eg. 'if (a & (a - 1))' would pass if a = 0b100, not if a = 0b110. Since:
> >> 	0b100 - 0b001 = 0b011 -> (0b100 & 0b011) => false
> >> 	0b110 - 0b001 = 0b101 -> (0b100 & 0b101) => true
> >
> > If it's common, maybe there should be a function for it that properly
> > reflects the intended behavior.
> >
>
> Not my decision but I would guess that the general consensus is that we
> don't want to obscure bit manipulation in helper functions.
>
> These patterns are often considered base knowledge and since it doesn't take
> up any space a "helper" function only obscures what's happening. In
> particular from a review perspective.
>
> So unless there exists a function for this pattern already I don't think
> it's something that merits replacing.
>
> That's my 2c on that subject.
>
> That said, there are certainly places where swapping in the
> is_power_of_2() function is applicable. It uses a fancy bit trick which
> wasn't as obvious to me. Generally the right side of the expression
> below is the one I would use and no helper function existed.
>
> 	(n - 1 < (n ^ (n - 1))) == (n && !(n & (n - 1)))
>
> So if you are looking into doing some janitor work in this area you
> should keep in mind that, without that zero guard 'n &&' swapping the
> function might be altering the logic.
>
> So, a heads up is all.

  Not to toot my own horn but if you go back through history, I was
the one who kicked off the introduction of the first "is power of 2"
helper function many, many years ago:


  commit 63c2f782e8f6aafbc11b14b2cb291b3dc9fc217d
  Author: Robert P. J. Day <rpjday@mindspring.com>
  Date:   Tue Jan 30 06:06:00 2007 -0500

    [POWERPC] Add "is_power_of_2" checking to log2.h.

    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@mindspring.com>
    Acked-by: Kumar Gala <galak@kernel.crashing.org>
    Signed-off-by: Paul Mackerras <paulus@samba.org>


  And, no, a lot of that content was not immediately obvious as a
candidate for simplification; when I wrote my regex searching scripts,
I deliberately made them overly general *knowing* I would get false
positives, then I manually checked whether they should be simplified.
It never occurred to me that some of those checks were actually asking
whether more than one bit flag was set, which semantically is asking a
different question.

  Anyway, people were asking about janitorial work so I threw out a
couple of ideas. Whether they're worth pursing ... not my call.

rday

  reply	other threads:[~2026-04-02 12:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 11:49 more potential janitor work: simplifying test for power of 2 Robert P. J. Day
2026-04-01 12:16 ` Linus Probert
2026-04-02  9:50   ` Linus Probert
2026-04-02 10:07     ` Julia Lawall
2026-04-02 11:57       ` Linus Probert
2026-04-02 12:40         ` Robert P. J. Day [this message]
2026-04-02 13:04           ` Linus Probert
2026-04-02 13:21             ` Robert P. J. Day

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bcd30f78-7cbb-4284-dc18-302b549a00e2@crashcourse.ca \
    --to=rpjday@crashcourse.ca \
    --cc=julia.lawall@inria.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linus.probert@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox