svn commit: r230230 - head/sys/dev/random

[Andrey Chernov]

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/