Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 18 Jun 2018 23:41:58 +0300 | Subject | Re: [PATCH v4 1/3] bitfield: fix *_encode_bits() |
| |
On Mon, Jun 18, 2018 at 11:37 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > There's a bug in *_encode_bits() in using ~field_multiplier() for > the check whether or not the constant value fits into the field, > this is wrong and clearly ~field_mask() was intended. This was > triggering for me for both constant and non-constant values. > > Additionally, make this case actually into an compile error. > Declaring the extern function that will never exist with just a > warning is pointless as then later we'll just get a link error. > > While at it, also fix the indentation in those lines I'm touching. > > Finally, as suggested by Andy Shevchenko, add some tests and for > that introduce also u8 helpers. The tests don't compile without > the fix, showing that it's necessary. >
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Fixes: 00b0c9b82663 ("Add primitives for manipulating bitfields both in host- and fixed-endian.") > Signed-off-by: Johannes Berg <johannes@sipsolutions.net> > --- > v2: replace stray tab by space > v3: u8 helpers, tests > v4: minor cleanup (Andy) > split into two patches (Andy) > > If you don't mind, I'd like to take this through the networking > tree(s) as a fix since I have some pending code that depends on > it. > --- > include/linux/bitfield.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h > index cf2588d81148..147a7bb341dd 100644 > --- a/include/linux/bitfield.h > +++ b/include/linux/bitfield.h > @@ -104,7 +104,7 @@ > (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \ > }) > > -extern void __compiletime_warning("value doesn't fit into mask") > +extern void __compiletime_error("value doesn't fit into mask") > __field_overflow(void); > extern void __compiletime_error("bad bitfield mask") > __bad_mask(void); > @@ -121,8 +121,8 @@ static __always_inline u64 field_mask(u64 field) > #define ____MAKE_OP(type,base,to,from) \ > static __always_inline __##type type##_encode_bits(base v, base field) \ > { \ > - if (__builtin_constant_p(v) && (v & ~field_multiplier(field))) \ > - __field_overflow(); \ > + if (__builtin_constant_p(v) && (v & ~field_mask(field))) \ > + __field_overflow(); \ > return to((v & field_mask(field)) * field_multiplier(field)); \ > } \ > static __always_inline __##type type##_replace_bits(__##type old, \ > -- > 2.14.4 >
-- With Best Regards, Andy Shevchenko
| |