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/>