>[email protected] wrote: > >> This patch solves two problems. >> >> First, abort if denom is greater than UINT32_MAX. arc4random_uniform >> expects an uint32_t. If floor(denom) is greater than UINT32_MAX then >> the cast is undefined behaviour. > >This isn't a very important program, but the points are valid because >we may learn something which applies in other places. > >So your change makes fail with error + stderr output for large >floating point numbers, rather than producing limited range values >(the range is incorrect, too small, but it still succeeds). You >make it more correct, but scripts using this would fail badly. That >bothers me.
Another way to solve this problem would be to trim the numbers with something like this: if (denom > UINT32_MAX) denom = UINT32_MAX. >I can understand why code was moved to arc4random(3) in 1997, and to >arc4random_uniform(3) in 2008. > >In 2014, I changed the rand/random/drand48 functions to be >non-deterministic by default. drand48(3) is a double-sized random >producer. Maybe we should move back to drand48, to gain the full >range? >Alternatively, copy the drand48.c code locally, in case it needs >some tweaks. > >However, using drand48() will mean using a floating point modulus. >We lose the uniform aspect. I'm not the non-uniform aspects are as >visible in the floating point range. Succeeding for the full floating >point range is more important than what arc4random_uniform() is trying >to do. But maybe a uniform version of the double code can grow out of >using the drand48.c code? Theo, I'm not sure I understand your reasoning. Are you trying to say that we should see if it's possible to create a drand48_uniform? -lucic71
