Re: [PATCH v2][BZ 14561] Separate rand and random seeds.

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/03/2013 06:25 PM, OndÅej BÃlka wrote:
> On Thu, May 23, 2013 at 06:10:59PM -0400, Carlos O'Donell wrote:
>> On 05/20/2013 03:06 PM, OndÅej BÃlka wrote:
>>> Hi, 
>>> Functions rand calls random which causes seed to be shared between them.
>>>
>>> This patch causes rand and random use separate seeds.
>>>
>>> Comments?
>>>
>>> Ondra
>>>
>>> 	* stdlib/rand.c: Include stdlib/random.c
>>> 	* stdlib/random.c: Separate rand and random.
>>
>> This is user visible change and needs a BZ#.
>>
>> This also needs a test case similar to what Rich posted
>> with a comment indicating the rationale from the standards
>> perspective.
>>
>> Repost with test, BZ#, and fixes below.
>>
> Here is v2

Thanks for creating the BZ.

Please don't forget to add "[BZ #14561]" in ChangeLog, and update NEWS.

> 	* stdlib/rand.c: Include stdlib/random.c
> 	* stdlib/random.c: Separate rand and random.
> 
> 
> ---
>  stdlib/rand.c   |   19 +++++++++++--------
>  stdlib/random.c |   30 +++++++++++++++++++++++-------
>  2 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/stdlib/rand.c b/stdlib/rand.c
> index 3e0839f..290e574 100644
> --- a/stdlib/rand.c
> +++ b/stdlib/rand.c
> @@ -15,14 +15,17 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdlib.h>
> -
> -#undef	rand
>  
>  
>  /* Return a random integer between 0 and RAND_MAX.  */
> -int
> -rand ()
> -{
> -  return (int) __random ();
> -}
> +
> +#define SRANDOM srand
> +#define __SRANDOM __srand
> +
> +#define RANDOM_RET int
> +#define RANDOM rand
> +#define __RANDOM __rand
> +
> +#define USE_AS_RAND
> +
> +#include "stdlib/random.c"
> diff --git a/stdlib/random.c b/stdlib/random.c
> index 967dec3..61483bd 100644
> --- a/stdlib/random.c
> +++ b/stdlib/random.c
> @@ -204,8 +204,14 @@ __libc_lock_define_initialized (static, lock)
>     information a given number of times to get rid of any initial dependencies
>     introduced by the L.C.R.N.G.  Note that the initialization of randtbl[]
>     for default usage relies on values produced by this routine.  */
> +
> +#ifndef USE_AS_RAND
> +# define __SRANDOM __srandom
> +# define SRANDOM srandom
> +#endif
> +
>  void
> -__srandom (x)
> +__SRANDOM (x)
>       unsigned int x;
>  {
>    __libc_lock_lock (lock);
> @@ -213,8 +219,10 @@ __srandom (x)
>    __libc_lock_unlock (lock);
>  }
>  
> -weak_alias (__srandom, srandom)
> -weak_alias (__srandom, srand)
> +weak_alias (__SRANDOM, SRANDOM)
> +
> +#ifndef USE_AS_RAND
> +
>  
>  /* Initialize the state information in the given array of N bytes for
>     future random number generation.  Based on the number of bytes we
> @@ -276,7 +284,9 @@ __setstate (arg_state)
>  }
>  
>  weak_alias (__setstate, setstate)
> -
> +
> +#endif
> +
>  /* If we are using the trivial TYPE_0 R.N.G., just do the old linear
>     congruential bit.  Otherwise, we do our fancy trinomial stuff, which is the
>     same in all the other cases due to all the global variables that have been
> @@ -288,8 +298,13 @@ weak_alias (__setstate, setstate)
>     rear pointers can't wrap on the same call by not testing the rear
>     pointer if the front one has wrapped.  Returns a 31-bit random number.  */
>  
> -long int
> -__random ()
> +#ifndef USE_AS_RAND
> +# define RANDOM_RET long int
> +# define RANDOM __random
> +#endif
> +
> +RANDOM_RET
> +RANDOM ()
>  {
>    int32_t retval;
>  
> @@ -301,5 +316,6 @@ __random ()
>  
>    return retval;
>  }
> -
> +#ifndef USE_AS_RAND
>  weak_alias (__random, random)
> +#endif
> 

This looks OK to me.

However, please make sure we get the testcase in to prevent
a regression.

Cheers,
Carlos.