Hello! On Sat, Dec 09, 2023 at 08:42:11AM +0000, J Carter wrote:
> On Sat, 09 Dec 2023 07:46:10 +0000 > J Carter <[email protected]> wrote: > > > # HG changeset patch > > # User J Carter <[email protected]> > > # Date 1702101635 0 > > # Sat Dec 09 06:00:35 2023 +0000 > > # Node ID 1a77698f82d2580aa8b8f62ce89b4dbb6d678c5d > > # Parent d9275e982a7188a1ea7855295ffa93362ea9830f > > Win32: extended ngx_random range to 0x7fffffff > > > > rand() is used on win32. RAND_MAX is implementation defined. win32's is > > 0x7fff. > > > > Existing uses of ngx_random rely upon 0x7fffffff range provided by > > posix implementations of random(). > > > > diff -r d9275e982a71 -r 1a77698f82d2 src/os/win32/ngx_win32_config.h > > --- a/src/os/win32/ngx_win32_config.h Sat Dec 09 05:09:07 2023 +0000 > > +++ b/src/os/win32/ngx_win32_config.h Sat Dec 09 06:00:35 2023 +0000 > > @@ -280,7 +280,9 @@ > > > > #define NGX_HAVE_GETADDRINFO 1 > > > > -#define ngx_random rand > > +#define ngx_random > > \ > > + ((rand() << 16) | (rand() << 1) | (rand() >> 14)) > > + > > #define ngx_debug_init() > > > > > > ^ my mistake - copying error.. > > # HG changeset patch > # User J Carter <[email protected]> > # Date 1702111094 0 > # Sat Dec 09 08:38:14 2023 +0000 > # Node ID 10ef59a412a330872fc6d46de64f42e32e997b3a > # Parent d9275e982a7188a1ea7855295ffa93362ea9830f > Win32: extended ngx_random range to 0x7fffffff Nitpicking: Win32: extended ngx_random() range to 0x7fffffff. > > rand() is used on win32. RAND_MAX is implementation defined. win32's is > 0x7fff. > > Existing uses of ngx_random rely upon 0x7fffffff range provided by > posix implementations of random(). Thanks for catching this. As far as I can see, the only module which actually relies on the range is the random index module. Relying on the ngx_random() range generally looks wrong to me, and we might want to change the code to don't. OTOH, it's the only way to get a completely uniform distribution, and that's what the module tries to do. As such, it might be good enough to preserve it as is, at least till further changes to ngx_random(). Either way, wider range for ngx_random() should be beneficial in other places. > > diff -r d9275e982a71 -r 10ef59a412a3 src/os/win32/ngx_win32_config.h > --- a/src/os/win32/ngx_win32_config.h Sat Dec 09 05:09:07 2023 +0000 > +++ b/src/os/win32/ngx_win32_config.h Sat Dec 09 08:38:14 2023 +0000 > @@ -280,7 +280,9 @@ > > #define NGX_HAVE_GETADDRINFO 1 > > -#define ngx_random rand > +#define ngx_random() > \ Nitpicking: the "\" character should be at the 79th column (in some files at 78th). This ensures that a diff won't wrap on a standard 80-column terminal. > + ((rand() << 16) | (rand() << 1) | (rand() >> 14)) > + > #define ngx_debug_init() Using "|" for random numbers looks utterly wrong to me, even if ORed values are guaranteed to not interfere. I would rather use "^", and avoid dependency on the particular value of RAND_MAX (other than POSIX-required minimum of 32767) by using something like 0x7fffffff & ((rand() << 16) ^ (rand() << 8) ^ rand()) with proper typecasts. Something like this should work: diff --git a/src/os/win32/ngx_win32_config.h b/src/os/win32/ngx_win32_config.h --- a/src/os/win32/ngx_win32_config.h +++ b/src/os/win32/ngx_win32_config.h @@ -280,7 +280,11 @@ typedef int sig_atomic_t #define NGX_HAVE_GETADDRINFO 1 -#define ngx_random rand +#define ngx_random() \ + ((long) (0x7fffffff & ( ((uint32_t) rand() << 16) \ + ^ ((uint32_t) rand() << 8) \ + ^ ((uint32_t) rand()) ))) + #define ngx_debug_init() -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list [email protected] https://mailman.nginx.org/mailman/listinfo/nginx-devel
