Re: random, srandom, and initstate

[Joern Rennecke <amylaar at spamcop dot net>]

Quoting Joern Rennecke <amylaar@spamcop.net>:

> Hmm, locking would seriously reduce the class of multithreading scenarious
> where random can work.  If an interrupt tried to use it while it is already
> active, deadlock would result - similar to malloc.
> OTOH it should avoid all the issues in having to change our data structures
> to support random (3) reentrancy - allowing us to use the NetBSD version
> with just minor changes.
> And it should be good enough for gcc.c-torture/unsorted/udivmod4.c and
> its ilk.

I've tried an implementation; tested on x86_64-unknown-linux-gnu X sh-elf
with gcc/testsuite/gcc.dg/vshift-{1,3}.c rolled back to revision 181613 .

Below is the diff to the NetBSD original, and the second attachment is the
patch for newlib.

I'm not sure about random_r / srandom_r : should I add a dummy struct _reent *
parameter so that the implementation could later/optionally be changed to
work without locking?

--- libc/stdlib/random.c-ORIG	2012-03-28 00:48:12.280125217 +0200
+++ libc/stdlib/random.c	2012-03-28 02:11:42.344120901 +0200
@@ -39,32 +39,29 @@ __RCSID("$NetBSD: random.c,v 1.2 2005/12
 #endif
 #endif /* LIBC_SCCS and not lint */
 
-#include "namespace.h"
 
 #include <assert.h>
 #include <errno.h>
 #include <stdlib.h>
-#include "reentrant.h"
-
-#ifdef __weak_alias
-__weak_alias(initstate,_initstate)
-__weak_alias(random,_random)
-__weak_alias(setstate,_setstate)
-__weak_alias(srandom,_srandom)
-#endif
-
+#include <reent.h>
+#include <sys/lock.h>
+#include <assert.h>
+#define _DIAGASSERT assert
 
-#ifdef _REENTRANT
-static mutex_t random_mutex = MUTEX_INITIALIZER;
-#endif
+#ifndef __SINGLE_THREAD__
+__LOCK_INIT(static,  random_mutex);
+#define mutex_lock(a)	__lock_acquire(a)
+#define mutex_unlock(a)	__lock_release(a)
 #else
-#include <lib/libkern/libkern.h>
 #define mutex_lock(a)	(void)0
 #define mutex_unlock(a) (void)0
 #endif
+#endif
 
-static void srandom_unlocked(unsigned int);
-static long random_unlocked(void);
+#ifdef _REENT_ONLY
+static
+#endif
+long random(void);
 
 #define USE_BETTER_RANDOM
 
@@ -91,7 +88,7 @@ static long random_unlocked(void);
  * state information, which will allow a degree seven polynomial.  (Note:
  * the zeroeth word of state information also has some other information
  * stored in it -- see setstate() for details).
- * 
+ *
  * The random number generation technique is a linear feedback shift register
  * approach, employing trinomials (since there are fewer terms to sum up that
  * way).  In this approach, the least significant bit of all the numbers in
@@ -262,8 +259,11 @@ static int *end_ptr = &randtbl[DEG_3 + 1
  * introduced by the L.C.R.N.G.  Note that the initialization of randtbl[]
  * for default usage relies on values produced by this routine.
  */
-static void
-srandom_unlocked(unsigned int x)
+#ifdef _REENT_ONLY
+static
+#endif
+void
+srandom(unsigned int x)
 {
 	int i;
 
@@ -296,16 +296,16 @@ srandom_unlocked(unsigned int x)
 		fptr = &state[rand_sep];
 		rptr = &state[0];
 		for (i = 0; i < 10 * rand_deg; i++)
-			(void)random_unlocked();
+			(void)random();
 	}
 }
 
 void
-srandom(unsigned long x)
+srandom_r(unsigned long x)
 {
 
 	mutex_lock(&random_mutex);
-	srandom_unlocked((unsigned int) x);
+	srandom((unsigned int) x);
 	mutex_unlock(&random_mutex);
 }
 
@@ -317,12 +317,12 @@ srandom(unsigned long x)
  * the break values for the different R.N.G.'s, we choose the best (largest)
  * one we can and set things up for it.  srandom() is then called to
  * initialize the state information.
- * 
+ *
  * Note that on return from srandom(), we set state[-1] to be the type
  * multiplexed with the current value of the rear pointer; this is so
  * successive calls to initstate() won't lose this information and will be
  * able to restart with setstate().
- * 
+ *
  * Note: the first thing we do is save the current state, if any, just like
  * setstate() so that it doesn't matter when initstate is called.
  *
@@ -376,7 +376,7 @@ initstate(
 	}
 	state = (int *) (int_arg_state + 1); /* first location */
 	end_ptr = &state[rand_deg];	/* must set end_ptr before srandom */
-	srandom_unlocked((unsigned int) seed);
+	srandom((unsigned int) seed);
 	if (rand_type == TYPE_0)
 		int_arg_state[0] = rand_type;
 	else
@@ -464,8 +464,11 @@ setstate(char *arg_state)		/* pointer to
  *
  * Returns a 31-bit random number.
  */
-static long
-random_unlocked(void)
+#ifdef _REENT_ONLY
+static
+#endif
+long
+random(void)
 {
 	int i;
 	int *f, *r;
@@ -495,12 +498,12 @@ random_unlocked(void)
 }
 
 long
-random(void)
+random_r(void)
 {
 	long r;
 
 	mutex_lock(&random_mutex);
-	r = random_unlocked();
+	r = random();
 	mutex_unlock(&random_mutex);
 	return (r);
 }

Attachment: random-diff-20120327
Description: Text document