Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e090466

Browse files
committed
Fix latent problem with pg_jrand48().
POSIX specifies that jrand48() returns a signed 32-bit value (in the range [-2^31, 2^31)), but our code was returning an unsigned 32-bit value (in the range [0, 2^32)). This doesn't actually matter to any existing call site, because they all cast the "long" result to int32 or uint32; but it will doubtless bite somebody in the future. To fix, cast the arithmetic result to int32 explicitly before the compiler widens it to long (if widening is needed). While at it, upgrade this file's far-short-of-project-style comments. Had there been some peer pressure to document pg_jrand48() properly, maybe this thinko wouldn't have gotten committed to begin with. Backpatch to v10 where pg_jrand48() was added, just in case somebody back-patches a fix that uses it and depends on the standard behavior. Discussion: https://postgr.es/m/17235.1545951602@sss.pgh.pa.us
1 parent 4ed6c07 commit e090466

File tree

1 file changed

+36
-6
lines changed

1 file changed

+36
-6
lines changed

src/port/erand48.c

+36-6
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,21 @@
22
*
33
* erand48.c
44
*
5-
* This file supplies pg_erand48(), pg_lrand48(), and pg_srand48(), which
6-
* are just like erand48(), lrand48(), and srand48() except that we use
7-
* our own implementation rather than the one provided by the operating
8-
* system. We used to test for an operating system version rather than
5+
* This file supplies pg_erand48() and related functions, which except
6+
* for the names are just like the POSIX-standard erand48() family.
7+
* (We don't supply the full set though, only the ones we have found use
8+
* for in Postgres. In particular, we do *not* implement lcong48(), so
9+
* that there is no need for the multiplier and addend to be variable.)
10+
*
11+
* We used to test for an operating system version rather than
912
* unconditionally using our own, but (1) some versions of Cygwin have a
1013
* buggy erand48() that always returns zero and (2) as of 2011, glibc's
1114
* erand48() is strangely coded to be almost-but-not-quite thread-safe,
1215
* which doesn't matter for the backend but is important for pgbench.
1316
*
17+
* Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
1418
*
15-
* Copyright (c) 1993 Martin Birgmeier
19+
* Portions Copyright (c) 1993 Martin Birgmeier
1620
* All rights reserved.
1721
*
1822
* You may redistribute unmodified or modified versions of this source
@@ -54,6 +58,9 @@ static unsigned short _rand48_mult[3] = {
5458
static unsigned short _rand48_add = RAND48_ADD;
5559

5660

61+
/*
62+
* Advance the 48-bit value stored in xseed[] to the next "random" number.
63+
*/
5764
static void
5865
_dorand48(unsigned short xseed[3])
5966
{
@@ -75,6 +82,10 @@ _dorand48(unsigned short xseed[3])
7582
}
7683

7784

85+
/*
86+
* Generate a random floating-point value using caller-supplied state.
87+
* Values are uniformly distributed over the interval [0.0, 1.0).
88+
*/
7889
double
7990
pg_erand48(unsigned short xseed[3])
8091
{
@@ -84,20 +95,39 @@ pg_erand48(unsigned short xseed[3])
8495
ldexp((double) xseed[2], -16);
8596
}
8697

98+
/*
99+
* Generate a random non-negative integral value using internal state.
100+
* Values are uniformly distributed over the interval [0, 2^31).
101+
*/
87102
long
88103
pg_lrand48(void)
89104
{
90105
_dorand48(_rand48_seed);
91106
return ((long) _rand48_seed[2] << 15) + ((long) _rand48_seed[1] >> 1);
92107
}
93108

109+
/*
110+
* Generate a random signed integral value using caller-supplied state.
111+
* Values are uniformly distributed over the interval [-2^31, 2^31).
112+
*/
94113
long
95114
pg_jrand48(unsigned short xseed[3])
96115
{
97116
_dorand48(xseed);
98-
return ((long) xseed[2] << 16) + ((long) xseed[1]);
117+
return (int32) (((uint32) xseed[2] << 16) + (uint32) xseed[1]);
99118
}
100119

120+
/*
121+
* Initialize the internal state using the given seed.
122+
*
123+
* Per POSIX, this uses only 32 bits from "seed" even if "long" is wider.
124+
* Hence, the set of possible seed values is smaller than it could be.
125+
* Better practice is to use caller-supplied state and initialize it with
126+
* random bits obtained from a high-quality source of random bits.
127+
*
128+
* Note: POSIX specifies a function seed48() that allows all 48 bits
129+
* of the internal state to be set, but we don't currently support that.
130+
*/
101131
void
102132
pg_srand48(long seed)
103133
{

0 commit comments

Comments
 (0)