Re: [PATCH] New module rand48 [rebased]

[Bruno Haible]

Hi Richard,

> Updated with feedback ...

The sources come from and are presumed to be shared with glibc, therefore
please mention this in the module description. Our convention is like this:

  Maintainer:
  Richard W.M. Jones, glibc

And, to make comparisons between glibc sources and gnulib sources easier,
please use the same file names as glibc does:
  drand48-iter.c
  drand48.c
  drand48_r.c
  erand48.c
  erand48_r.c
  jrand48.c
  jrand48_r.c
  lrand48.c
  lrand48_r.c
  mrand48.c
  mrand48_r.c
  nrand48.c
  nrand48_r.c
  srand48.c
  srand48_r.c

> +int
> +__drand48_iterate (xsubi, buffer)

It's a bad idea to define functions starting with __ outside libc, because
such identifiers are in the system's "namespace". As a workaround, you can
add a
  #define __drand48_iterate _gl_drand48_iterate
to stdlib.in.h.

> +#ifdef HAVE_IEEE754_H
> +# include <ieee754.h>
> +#else
> +# if defined(__i386__)
> +
> +#define IEEE754_DOUBLE_BIAS 0x3ff
> +
> +union ieee754_double
> +{
> +  double d;
> +
> +  /* This is only correct on i386 & x86-64 systems. */
> +  struct
> +  {
> +    unsigned int mantissa1:32;
> +    unsigned int mantissa0:20;
> +    unsigned int exponent:11;
> +    unsigned int negative:1;
> +  } ieee;
> +};
> +# endif /* __i386__ */
> +#endif /* !HAVE_IEEE754_H */
> +
> +int
> +erand48_r (xsubi, buffer, result)
> +     unsigned short int xsubi[3];
> +     struct drand48_data *buffer;
> +     double *result;
> +{
> +  union ieee754_double temp;

How is this meant to compile on non-x86 CPUs? glibc is the only system to
have a <ieee754.h>. To make this portable, I think, you need to combine up
to 4 uint32_t values to a 'double' in the range 0..1-epsilon, e.g.

  uint32_t value1 = ...;
  uint32_t value2 = ...;
  uint32_t value3 = ...;
  uint32_t value4 = ...;
  static const double t32 = /* 2^-32 */
    (1.0 / 65536.0) * (1.0 / 65536.0);
  double x = (value1 + (value2 + (value3 + value4 * t32) * t32) * t32) * t32;

> +    unsigned long long int __a; /* Factor in congruential formula.  */

'unsigned long long' is not supported by all compilers, e.g. MSVC. Better
use uint64_t instead.

Bruno