On Thu, Dec 21, 2023 at 07:14:40PM +0300, Maxim Dounin wrote: > Hello! > > On Thu, Dec 21, 2023 at 05:37:02PM +0400, Sergey Kandaurov wrote: > > > > On 16 Dec 2023, at 06:57, Maxim Dounin <mdou...@mdounin.ru> wrote: > > > > > > 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 <jordanc.car...@outlook.com> wrote: > > >> > > >>> # HG changeset patch > > >>> # User J Carter <jordanc.car...@outlook.com> > > >>> # 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 <jordanc.car...@outlook.com> > > >> # 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() > > > > > > > > > > Nitpicking: you might want to re-align the "^" operator to the first > > symbol of the left-hand operand (similar to NGX_CONF_TAKE1234, or > > even NGX_UNIX_ADDRSTRLEN). Other than that, it looks good. > > It's intentionally aligned this way to simplify reading. Such > style is occasionally used in complex macro definitions, see > ngx_mp4_get_32value() or ngx_proxy_protocol_parse_uint32() for > some examples. > > Just for completeness, below is the updated patch. > > # HG changeset patch > # User J Carter <jordanc.car...@outlook.com> > # Date 1702111094 0 > # Sat Dec 09 08:38:14 2023 +0000 > # Node ID 92923ac5ea2a395774b28460f07d0fd2e1a2de24 > # Parent cc16989c6d61385027c1ebfd43929f8369fa5f62 > Win32: extended ngx_random() range to 0x7fffffff. > > rand() is used on win32. RAND_MAX is implementation defined. win32's is > 0x7fff.
I'd unwrap this line to look more pleasant, it fits into 80 columns. > > Existing uses of ngx_random() rely upon 0x7fffffff range provided by > POSIX implementations of random(). > > diff -r cc16989c6d61 -r 92923ac5ea2a src/os/win32/ngx_win32_config.h > --- a/src/os/win32/ngx_win32_config.h Sat Dec 16 03:40:01 2023 +0400 > +++ b/src/os/win32/ngx_win32_config.h Sat Dec 09 08:38:14 2023 +0000 > @@ -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) > \ If I'm not mistaken, indentation is now 5 spaces. Otherwise, looks good. > + ^ ((uint32_t) rand() << 8) > \ > + ^ ((uint32_t) rand()) ))) > + > #define ngx_debug_init() > > > _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel