Re: more potential janitor work: simplifying test for power of 2
"Robert P. J. Day" <[email protected]>
| Newsgroups | org.kernel.vger.kernel-janitors |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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 <[email protected]> Acked-by: Kumar Gala <[email protected]> Signed-off-by: Paul Mackerras <[email protected]> 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