Re: [PATCH] random_r: new module

[Bruno Haible]

Jim Meyering wrote:
> I saw that there was no random-number generating module in gnulib,
> and certainly nothing reentrant, so took glibc's random_r.c and did this:

Very nice!

> diff --git a/doc/glibc-functions/random_r.texi 
> b/doc/glibc-functions/random_r.texi
> index 50081b6..96f9384 100644
> --- a/doc/glibc-functions/random_r.texi
> +++ b/doc/glibc-functions/random_r.texi
> @@ -2,7 +2,7 @@
>  @subsection @code{random_r}
>  @findex random_r
> 
> -Gnulib module: ---
> +Gnulib module: random_r
> 
>  Portability problems fixed by Gnulib:
>  @itemize

Together with the mention of the modules, you can move the
"This function is missing ..." problem from the "not fixed by Gnulib" to the
"fixed by Gnulib" section.

random_r.c should #include <config.h> and <inttypes.h> (for int32_t).

> +int
> +__initstate_r (unsigned int seed, char *arg_state, size_t n,
> +            struct random_data *buf)

This function has 4 variable declarations that follow statements (C99,
not ANSI C).

> +# undef random_r
> +# define random_r(r) \
> +    (GL_LINK_WARNING ("random_r is unportable - " \
> +                      "use gnulib module random_r for portability"), \
> +     random_r (r))
> +# undef initstate_r
> +# define initstate_r(r) \
> +    (GL_LINK_WARNING ("initstate_r is unportable - " \
> +                      "use gnulib module random_r for portability"), \
> +     initstate_r (r))
> +# undef srandom_r
> +# define srandom_r(r) \
> +    (GL_LINK_WARNING ("srandom_r is unportable - " \
> +                      "use gnulib module random_r for portability"), \
> +     srandom_r (r))
> +# undef setstate_r
> +# define setstate_r(r) \
> +    (GL_LINK_WARNING ("setstate_r is unportable - " \
> +                      "use gnulib module random_r for portability"), \
> +     setstate_r (r))

In these macros, the number of arguments should be the same as for the
corresponding function, otherwise the compiler will yell "too many arguments
for macro".

> +Depends-on:
> +errno

The dependence to the 'errno' module is not needed, since the only used
error value, EINVAL, is portable. (See doc/posix-headers/errno.texi for
complete details.)

> +Makefile.am:
> +lib_SOURCES += random_r.c

This will cause random_r.c to be compiled on all platforms, including glibc
platforms. But you are actually activating it through AC_LIBOBJ. So this line
can be removed.

> +  if (initstate_r (time (0), buf, sizeof buf, &rand_state))

By habit to use NULL for pointers and 0 for numbers, I would write
'time (NULL)' here. But that merely a question of style.

Bruno