Re: Patch: rework kernel random number subsystem

[Thor Lancelot Simon <tls%panix.com@localhost>]

On Sat, Oct 22, 2011 at 04:35:42PM +0000, Christos Zoulas wrote:
> 
> 1) +  memset(r, 0, sizeof(r)); needs (*r)

Indeed.

> 2) The code around the above memset has whitespace issues.

There are a lot of KNF issues in general.  I'll fix these in a final
pass before I check anything in, and send another patch so folks can
confirm that they've been dealt with.

I tried not to make non-whitespace KNF changes to the externally
sourced code (rngtest.c, which is Greg Rose's "fips140.c", and the
ctr_drbg code, which is hardly modified at all from the original
source distribution) to make integrating any later changes easier.
But perhaps there won't be any.  Should I re-indent all that code
and otherwise KNF it?

> 3) Why do we have loops that have both a count sentinel and the list pointer
>    sentinel? Shouldn't/couldn't those always be synced?

See below.

> 4) What's the hardcoded 16 in the name compares?

I'll fix.

> 5) sizeof(type) in memcpy() should be sizeof(*dst

Same bug as #1, but with worse consequences.  Oops.

> 6) Isn't it possible to use the list foreach macros instead of open-coding?

Regarding this, and #3, I made a deliberate effort to _not_ convert all
the rnd.c code from open coded loops over the queue.h datastructures to
_FOREACH at this juncture.  A few of the instances I checked exited the
loop in ways or had intentional side-effects that would have meant more
extensive code changes to use _FOREACH and I did not want to risk new and
exciting bugs from a wholesale rototill.

I am thinking it would be best to stabilize my changes, do the other
work in rnd.c I had slated for my "second step", then address this
kind of stylistic issue.  Is that OK?

> 7) instead of printfs shouldn't we use aprint?

I am not sure.  The documentation says aprint is for autoconfig only.
Is it really intended to be used later in the kernel run or by code
outside sys/dev or sys/*/*/dev?

Thanks for looking at the patch!  I know it's rather unwieldy.

Thor