Re: [PATCH treewide v2 1/3] bitfield: Add non-constant field_{prep,get}() helpers

From: Johannes Berg
Date: Mon Feb 03 2025 - 02:44:56 EST


On Sun, 2025-02-02 at 12:53 -0500, Yury Norov wrote:
>
> > Instead of creating another variant for
> > non-constant bitfields, wouldn't it be better to make the existing macro
> > accept both?
>
> Yes, it would definitely be better IMO.

On the flip side, there have been discussions in the past (though I
think not all, if any, on the list(s)) about the argument order. Since
the value is typically not a constant, requiring the mask to be a
constant has ensured that the argument order isn't as easily mixed up as
otherwise.

With a non-constant mask there can also be no validation that the mask
is contiguous etc.

Now that doesn't imply a strong objection - personally I've come to
prefer the lower-case typed versions anyway - but something to keep in
mind when doing this.

However, the suggested change to BUILD_BUG_ON_NOT_POWER_OF_2 almost
certainly shouldn't be done for the same reason - not compiling for non-
constant values is [IMHO] part of the API contract for that macro. This
can be important for the same reasons.

(Obviously, doing that change now doesn't invalidate existing code, but
it does remove checks that may have been intended to be present in the
code.)

johannes