Messages in this thread | | | Subject | Re: [PATCH] bitfield.h: add FIELD_MAX() and field_max() | From | Alex Elder <> | Date | Sat, 29 Feb 2020 07:10:45 -0600 |
| |
On 2/28/20 12:04 PM, Alex Elder wrote: > On 2/28/20 11:56 AM, Jakub Kicinski wrote: >> On Fri, 28 Feb 2020 10:53:43 -0600 Alex Elder wrote: >>> Define FIELD_MAX(), which supplies the maximum value that can be >>> represented by a field value. Define field_max() as well, to go >>> along with the lower-case forms of the field mask functions. >>> >>> Signed-off-by: Alex Elder <elder@linaro.org> >>> --- >>> >>> NOTE: I'm not entirely sure who owns/maintains this file so >>> I'm sending it to those who have committed things to it. >>> I hope someone will just take it in after review; I use >>> field_max() in some code I'm about to send out. >> >> Could you give us an example use? >> >> Is it that you find the current macros misnamed or there's something >> you can't do? Or are you trying to set fields to max? > > I'm trying to validate variable values are in range before attempting > to use them in a bitfield.
I should have actually checked my code before I sent this. Yes I am using the macro as I described, to see if something fits. But I'm also using it this way:
foo = u32_get_bits(register, FOO_COUNT_FMASK); if (foo == field_max(FOO_COUNT_MASK)) ; /* This has special meaning */
And another way:
size_limit = field_max(FOO_COUNT_MASK) * sizeof(struct foo);
So field_max() is really what I need here. It does imply a signed interpretation of the field value, but that's true for all of the lower-case bitfield functions.
I understand the value of FIELD_FIT() but I think field_max() (and FIELD_MAX()) serves a purpose. In fact one could argue it makes FIELD_FIT() unnecessary (compare against field_max(mask) instead) but I won't propose removing it.
So after further consideration I believe the original patch is fine. What are your thoughts?
-Alex
> I find field_max() to be a good name for what I'm looking for. > > -Alex > > #define FOO_FMASK 0x000ff000 > > static u32 register = 0x12345678; > > int foo(u32 value) > { > if (value > field_max(FOO_FMASK)) > return -EINVAL; > > u32_replace_bits(®ister, value, FOO_FMASK); > > return 0; > } >
| |