Date: Fri, 27 Jan 2012 20:34:35 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andrey Chernov <ache@FreeBSD.org> Cc: src-committers@FreeBSD.org, John Baldwin <jhb@FreeBSD.org>, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org, David Schultz <das@FreeBSD.org>, Mark Murray <markm@FreeBSD.org> Subject: Re: svn commit: r230230 - head/sys/dev/random Message-ID: <20120127201855.K1604@besplex.bde.org> In-Reply-To: <20120126175021.GA93016@vniz.net> References: <E1Rny2A-000C3x-O6@groundzero.grondar.org> <20120126143819.GA88677@vniz.net> <20120126155626.GA92229@vniz.net> <201201261132.38320.jhb@freebsd.org> <20120126165521.GA92622@vniz.net> <20120126175021.GA93016@vniz.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 26 Jan 2012, Andrey Chernov wrote:
>> On Thu, Jan 26, 2012 at 11:32:38AM -0500, John Baldwin wrote:
>>> Atomics don't operate on enums. You'll need to make it an int and just use
>>> #define's for the 3 states.
This restores some style bugs and keeps old ones.
> --- sys/libkern.h.old 2012-01-16 07:15:12.000000000 +0400
> +++ sys/libkern.h 2012-01-26 21:40:21.000000000 +0400
> @@ -72,6 +72,10 @@ static __inline quad_t qabs(quad_t a) {
>
> /* Prototypes for non-quad routines. */
> struct malloc_type;
> +#define ARC4_ENTR_NONE 0 /* Don't have entropy yet. */
> +#define ARC4_ENTR_HAVE 1 /* Have entropy. */
> +#define ARC4_ENTR_SEED 2 /* Reseeding. */
> +extern volatile int arc4rand_iniseed_state;
I see no prototypes here.
> uint32_t arc4random(void);
> void arc4rand(void *ptr, u_int len, int reseed);
I see a prototype with style bugs here (names for parameters, which isn't
bug for bug compatible with the other prototypes).
> int bcmp(const void *, const void *, size_t);
> --- dev/random/randomdev_soft.c.old 2011-03-02 01:42:19.000000000 +0300
> +++ dev/random/randomdev_soft.c 2012-01-26 19:35:12.000000000 +0400
> @@ -366,6 +366,8 @@ random_yarrow_unblock(void)
> selwakeuppri(&random_systat.rsel, PUSER);
> wakeup(&random_systat);
> }
> + (void)atomic_cmpset_int(&arc4rand_iniseed_state,
> + ARC4_ENTR_NONE, ARC4_ENTR_HAVE);
> }
This is gnu style. Continuation indentation is 4 spaces in KNF.
>
> static int
> --- libkern/arc4random.c.old 2008-08-08 01:51:09.000000000 +0400
> +++ libkern/arc4random.c 2012-01-26 21:40:47.000000000 +0400
> @@ -24,6 +24,8 @@ __FBSDID("$FreeBSD: src/sys/libkern/arc4
> #define ARC4_RESEED_SECONDS 300
> #define ARC4_KEYBYTES (256 / 8)
>
> +volatile int arc4rand_iniseed_state = ARC4_ENTR_NONE;
> +
I don't see what all the volatile qualifiers and atomic ops are for.
The volatiles certainly have no effect, since this variable is only
accessed by atomic ops which only access variables as volatile whether
you want this or not. See das's reply for more about the atomic ops.
Here I think they just close a window that would be open just once for
each for a few instructions while booting.
> static u_int8_t arc4_i, arc4_j;
> static int arc4_numruns = 0;
> static u_int8_t arc4_sbox[256];
> @@ -130,7 +132,9 @@ arc4rand(void *ptr, u_int len, int resee
> struct timeval tv;
>
> getmicrouptime(&tv);
> - if (reseed ||
> + if (atomic_cmpset_int(&arc4rand_iniseed_state,
> + ARC4_ENTR_HAVE, ARC4_ENTR_SEED) ||
Gnu style indentation. Excessively early line splitting.
> + reseed ||
> (arc4_numruns > ARC4_RESEED_BYTES) ||
> (tv.tv_sec > arc4_t_reseed))
Old code uses KNF style (except for excessive line splitting and excessive
parentheses) in the same statement.
> arc4_randomstir();
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120127201855.K1604>