svn commit: r307148 - in head/lib/libc: gen stdlib

[Ed Maste]

On 13 October 2016 at 02:05, Bruce Evans <brde at optusnet.com.au> wrote:
> On Wed, 12 Oct 2016, Ed Maste wrote:
>
> The comment starts by being just wrong:
>
> 1. "The" sysctl is not used here.  Instead, a wrapper arc4_sysctl() is used.
>    The wrapper handles some but not all errors.

Fixed in my WIP tree.

> 2. The sysctl can and does fail.  It fails on:
>    - all old kernels mixed with new userlands

We don't support new userlands on very old kernels, and I think there
are many other things in libc that will fail on kernels old enough to
lack kern.arandom.

>    - with new kernels, at boot time, before the random device is seeded.

If that is indeed still possible it's a bug we need to fix before 12.0.

> 3. The sysctl can, or at least used to, return short reads with nonzero
>    counts.

That was addressed in markm's 2015 random work, I think. Presumably
random() was silently broken for the rand_type != TYPE_0 case prior to
that.

>    The documentation for this is well hidden, but the
>    arc4_sysctl() wrapper exists to support short reads, or perhaps just
>    the special case of short reads of 0, which it handles poorly by
>    possibly spinning forever.

I suspect we can just remove the arc4_sysctl wrapper too.

> Then the excuse is wrong.  abort() never makes sense in library functions.

arc4random must not return without good quality random data. The other
option would be for it to loop indefinitely.

> Here it gives very confusing errors for the delicate boot-time fandago
> case.

The "delicate boot-time fandango case" was a bug.

> Style bugs:
> - sentence breaks are 2 spaces in KNF, and all old code in this file follows
>   that rule.

Fixed in my WIP tree.

> - 'abort' is not marked up

Fixed in my WIP tree.

> This is even more broken, since it doesn't have the wrapper.

This and the other issues predate my changes; I'll take a look at the
history soon.