Hi,
Le 14/11/2022 à 19:38, Jason A. Donenfeld a écrit :
On Mon, Nov 14, 2022 at 07:04:13PM +0100, Yann Droneaud wrote:
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).
This is one of those bikeshed things you see all over the place, like
whether slices in a language should be [start index, end index] or
[start index, length], or whether arrays should be 0-based or 1-based.
We'll never settle this variety of dispute here.
But in this case, there are some particular reasons why it must be this
way. Firstly, usage of it this way matches most of the ways the function
is actually used in the kernel, and fits existing semantics. This alone
I find compelling. But also, having all of these functions use half-open
intervals means that each function can take care of its entire range,
without having to resort to using 64-bit arithmetic, and no function is
a complete subset of any other function. So doing it this way makes
these maximally useful too.
For get_random_below(), which replaces a modulo, <bikeshedding> and could
have been called get_random_mod()</bikeshedding>, having an open upper
range seems fine. It's already what can be achieved by the % operator.
But I believe it's unfortunate get_random_between() cannot be called to
get a number up to UINT32_MAX, as get_random_between(0, UINT32_MAX) would
be capped to UINT32_MAX - 1.
When not a constant, one could hope the function can cope with a maximum
that would grow up to and including UINT32_MAX.
So anyway I think the function has to be defined like this. If you'd
like to bikeshed over a different name than "between", though, be my
guest. Maybe you'd like "from" better. But probably "between" is fine,
and with enough good examples (as my conversion patch does) and the
clear succinct documentation comment, we should be good.
That the conversion patch [1] that triggered my comment: I find replacing
the following rather unpleasing, somewhat uncanny:
-get_random_u32_below(1024) + 1 + get_random_u32_between(1, 1024 + 1) I
would prefer
- get_random_u32_below(1024) + 1 + get_random_u32_between(1, 1024) [1]https://lore.kernel.org/lkml/20221114164558.1180362-4-Jason@xxxxxxxxx/
Regards.
--
Yann Droneaud
OPTEYA
