Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> How did Knuth get into this? This algorithm is certainly not his,
>> so why are those constants at all relevant?
> They are not more nor less relevant than any other "random" constant. The
> state needs a default initialization. The point of using DK's is that it
> is somehow cannot be some specially crafted value which would have some
> special property only know to the purveyor of the constant and could be
> used by them to break the algorithm.
Well, none of that is in the comment, which is probably just as well
because it reads like baseless paranoia. *Any* initialization vector
should be as good as any other; if it's not, that's an algorithm fault.
(OK, I'll give it a pass for zeroes being bad, but otherwise not.)
>> * Function names like these convey practically nothing to readers:
>>
>> +extern int64 pg_prng_i64(pg_prng_state *state);
>> +extern uint32 pg_prng_u32(pg_prng_state *state);
>> +extern int32 pg_prng_i32(pg_prng_state *state);
>> +extern double pg_prng_f64(pg_prng_state *state);
>> +extern bool pg_prng_bool(pg_prng_state *state);
> The intention is obviously "postgres pseudo-random number generator for
> <type>". ISTM that it conveys (1) that it is a postgres-specific stuff,
> (2) that it is a PRNG (which I find *MUCH* more informative than the
> misleading statement that something is random when it is not, and it is
> shorter) and (3) about the type it returns, because C does require
> functions to have distinct names.
> What would you suggest?
We have names for these types, and those abbreviations are (mostly)
not them. Name-wise I'd be all right with pg_prng_int64 and so on,
but I still expect that these functions' header comments should be
explicit about uniformity and about the precise output range.
As an example, it's far from obvious whether the minimum value
of pg_prng_int32 should be zero or INT_MIN. (Actually, I suspect
you ought to provide both of those cases.) And the output range
of pg_prng_float8 is not merely unobvious, but not very easy
to deduce from examining the code either; not that users should
have to.
>> BTW, why are we bothering with FIRST_BIT_MASK in the first place,
>> rather than returning "v & 1" for pg_prng_bool?
> Because some PRNG are very bad in the low bits, not xoroshiro stuff,
> though.
Good, but then you shouldn't write associated code as if that's still
a problem, because you'll cause other people to think it's still a
problem and write equally contorted code elsewhere. "v & 1" is a
transparent way of producing a bool, while this code isn't.
regards, tom lane