On Wed Sep 27 18:23:57 2017, each@isc.org wrote:
> >  - you removed the ifdef SC_PLATFORM_CRYPTORANDOM
> >   in dst__entropy_getdata. Please put it back as it simplifies
> >   the code and makes sure that dst library never uses a
> >   not crypto library PNRG. BTW don't confuse
> >   isc_entropy_usehook and isc_entropy_sethook, only
> >   the first changes the source of entropy.
> 
> I removed it because, when we're running with cryptorandom,
> isc_entropy_getdata() is hooked to dst_random_getdata() anyway,

=> it is not true: isc_entropy_usehook() is called *only* if
no random file / random dev is given or configured.
BTW without this #ifdef dnssec-keygen -r xxx is no longer
a no-op as described in the documentation.

> and if
> we're *not* running with cryptorandom, dst_random_getdata() returns
> NOTIMPLEMENTED, which means it's best if dst_random_getdata() isn't
> called directly.

=> the by-pass is consistent so I don't buy this argument.

> Doing it the way I did seems safer and cleaner, and it only adds the
> overhead of a single function call.

=> unfortunately it is not true and it makes dst behavior
relying on external to dst state.

> Come to think of it, I'd prefer it if it were called dst__random_getdata,
> to discourage people from relying on it or calling it directly. Just use
> it as an internal hook function only.

=> dst__random_getdata has the two __ to mark it as internal to dst.

> If you absolutely insist, I'll put it back the way it was, but I really
> do think this is architecturally better.

=> I absolutely insist.