[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Patch: rework kernel random number subsystem
On Sat, Oct 22, 2011 at 04:35:42PM +0000, Christos Zoulas wrote:
> 1) + memset(r, 0, sizeof(r)); needs (*r)
> 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?
> 4) What's the hardcoded 16 in the name compares?
> 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.
Main Index |
Thread Index |