Messages in this thread | | | Date | Mon, 14 Nov 2022 19:04:13 +0100 | From | Yann Droneaud <> | Subject | Re: [PATCH v1 3/5] random: add helpers for random numbers with given floor or range |
| |
[resent in text only]
Hi,
Le 22/10/2022 à 03:44, Jason A. Donenfeld a écrit : > Now that we have get_random_u32_below(), it's trivial to make inline > helpers to compute get_random_u32_above() and get_random_u32_between(), > which will help clean up open coded loops and manual computations > throughout the tree. > > Signed-off-by: Jason A. Donenfeld<Jason@zx2c4.com> > --- > include/linux/random.h | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/include/linux/random.h b/include/linux/random.h > index 3a82c0a8bc46..92188a74e50e 100644 > --- a/include/linux/random.h > +++ b/include/linux/random.h > @@ -91,6 +91,30 @@ static inline u32 get_random_u32_below(u32 ceil) > } > } > > +/* > + * Returns a random integer in the interval (floor, U32_MAX], with uniform > + * distribution, suitable for all uses. Fastest when floor is a constant, but > + * still fast for variable floor as well. > + */ > +static inline u32 get_random_u32_above(u32 floor) > +{ > + BUILD_BUG_ON_MSG(__builtin_constant_p(floor) && floor == U32_MAX, > + "get_random_u32_above() must take floor < U32_MAX"); > + return floor + 1 + get_random_u32_below(U32_MAX - floor); > +} > + > +/* > + * Returns a random integer in the interval [floor, ceil), with uniform > + * distribution, suitable for all uses. Fastest when floor and ceil are > + * constant, but still fast for variable floor and ceil as well. > + */ > +static inline u32 get_random_u32_between(u32 floor, u32 ceil) > +{ > + BUILD_BUG_ON_MSG(__builtin_constant_p(floor) && __builtin_constant_p(ceil) && > + floor >= ceil, "get_random_u32_above() must take floor < ceil"); > + return floor + get_random_u32_below(ceil - floor); > +}
I have a bad feeling about this one, and can't help but thinking it's going to bite someone: when asked to pick a number *between* 0 and 10, I usually think I'm allowed to pick 10 (even if I'm going to answer 7 as it should).
Also, regardinghttps://lore.kernel.org/lkml/20221114164558.1180362-4-Jason@zx2c4.com/ where there's a lot of such changes:
- seqno = get_random_u32_below(data->fc.chain_length) + 1; + seqno = get_random_u32_between(1, data->fc.chain_length + 1); IMHO, having the function returning a value in range [floor, ceil] would simplify the patch: - seqno = get_random_u32_below(data->fc.chain_length) + 1; + seqno = get_random_u32_between(1, data->fc.chain_length);
Regards.
-- Yann Droneaud OPTEYA
| |