Hi Yann, On 12/30/22 21:33, Alejandro Colomar wrote:
On 12/30/22 21:18, Yann Droneaud wrote:What's wrong with the following ?
[...]
unsigned long max = upper_bound - 1; unsigned long mask = ULONG_MAX >> __builtin_clzl(max);I hate coding these magic operations out of a function, when I can give it a meaningful name. That reads to me as a magic trick that many maintainers that read it after me will blame me for having to parse it.Moreover, it requires you to have the special case for 0 at the top, which I don't want.
I reconsidered; my -1 was equally magic. And by calling it 'mask', ULONG_MAX >> n is something not so magic.The builtin still has the problem that it requires special-casing 0, so I prefer the C23 call, which provides the behavior I want for 0:
unsigned long
shadow_random_uniform(unsigned long upper_bound)
{
unsigned long r, max, mask;
max = upper_bound - 1;
mask = ULONG_MAX >> leading_zerosul(max);
do {
r = shadow_random();
r &= mask; // optimization
} while (r > max);
return r;
}
And, now I don't need to add a wrapper around bit_ceil() that removes the UB.
stdc_leading_zerosul() is just fine for this use case.
Cheers, Alex -- <http://www.alejandro-colomar.es/>
OpenPGP_signature
Description: OpenPGP digital signature
