Date: Sat, 28 Jan 2012 08:31:18 +0400 From: Andrey Chernov <ache@FreeBSD.ORG> To: Bruce Evans <brde@optusnet.com.au> 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: <20120128043118.GA12241@vniz.net> In-Reply-To: <20120127201855.K1604@besplex.bde.org> 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> <20120127201855.K1604@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jan 27, 2012 at 08:34:35PM +1100, Bruce Evans wrote: > 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. About old bugs - I don't want this patch to be intrusive and touch style of old things. It can be committed as separate patch. > > /* 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. Will be moved before the comment. > > 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). I don't wan't to touch old things for now. > > 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. Will be 4-space indented. > > +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. I will remove volatile. About atomic ops: arc4rand calls are protected with mutex and yarrow calls are protected, but they can works concurrently wich each other. So, if we have atomic ops, why not to use them to close one time window too? > > + if (atomic_cmpset_int(&arc4rand_iniseed_state, > > + ARC4_ENTR_HAVE, ARC4_ENTR_SEED) || > > Gnu style indentation. Excessively early line splitting. Will be fixed. > Old code uses KNF style (except for excessive line splitting and excessive > parentheses) in the same statement. Old code is not in question at this moment. Thanx. -- http://ache.vniz.net/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120128043118.GA12241>