Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATCH: random: Fix some bugs in the random module



On 5/14/2026 10:20, Mikael Magnusson wrote:
% echo $(( zrand_int(4294967295) ))
zsh: Upper bound (4294967295) out of range: 0-4294967295
3910196459

read loop had incorrect condition as well as passing negative status
codes on to the pointer arithmetic

clz64 shifted by 1 bit instead of 2

remove hilariously incorrect comment
I'm no stats major.  But thank you for the clean up.

and some minor stuff
---
  Doc/Zsh/mod_random.yo     | 16 ++++++++-------
  Src/Modules/random.c      | 43 +++++++++++++++++++++------------------
  Src/Modules/random_real.c |  2 +-
  3 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/Doc/Zsh/mod_random.yo b/Doc/Zsh/mod_random.yo
index d797ca381b..f36da99662 100644
--- a/Doc/Zsh/mod_random.yo
+++ b/Doc/Zsh/mod_random.yo
@@ -25,21 +25,23 @@ enditem()
startitem()
  item(tt(zrand_int)+LPAR()tt(upper), tt(lower), tt(inclusive)RPAR()) (
-Returns a random integer between tt(lower) and tt(upper). All parameters are
-optional.  If none are specified it is equivalent to
-tt(SRANDOM).
+Returns a random integer between tt(lower) and tt(upper). All parameters
+are optional.  If none are specified it is equivalent to tt(SRANDOM). Note
+that this is equivalent to passing the default values except that it
+is inclusive.
tt(upper) is the upper bound on the resultant number and defaults to
-4,294,967,295.
+4,294,967,295. It is exclusive by default, unless tt(inclusive) is passed.
tt(lower) is the lower bound and defaults to 0. The defaults of these two arguments are also the maximum and minimum to which
  either can be set.
-tt(inclusive) is a flag that controls whether the result is ever equal to
-tt(upper).  By default it is not. If this argument is set to a non-zero value
-then it may be.
+tt(inclusive) is a flag that controls whether the tt(upper) bound is
+exclusive or inclusive. Passing any non-zero value makes it inclusive,
+otherwise it is exclusive when the tt(upper) bound is specified. The
+tt(lower) bound is always inclusive.
This is to facilitate a construct like tt($a[zrand_int+LPAR()$#a+RPAR()+1]) rather
  than tt($a[zrand_int+LPAR()$#a-1+RPAR()+1]).
diff --git a/Src/Modules/random.c b/Src/Modules/random.c
index 88ac9543c1..8aca7e9d4b 100644
--- a/Src/Modules/random.c
+++ b/Src/Modules/random.c
@@ -82,12 +82,15 @@ getrandom_buffer(void *buf, size_t len)
  		zwarn("Unable to get random data: %e.", errno);
  		return -1;
  	    }
+	    continue;
  	}
-#ifndef HAVE_ARC4RANDOM_BUF
+#ifdef HAVE_ARC4RANDOM_BUF
+    } while (0);
+#else
  	bufptr += ret;
  	val    += ret;
+    } while (val < len);
  #endif
-    } while (ret < len);
      return ret;
  }
@@ -110,17 +113,11 @@ get_bound_random_buffer(uint32_t *buffer, size_t count, uint32_t max)
      size_t i; /* loop counter */
getrandom_buffer((void*) buffer, count*sizeof(uint32_t));
-    if (max == UINT32_MAX)
-	return;
for(i=0;i<count;i++) {
  	multi_result = ((uint64_t) buffer[i]) * (uint64_t) max;
  	leftover = (uint32_t) multi_result;
- /*
-	 * The following if statement should (according to Google's Gemini)
-	 * only be executed with a probability of 1/2**32 or 2.33e-10
-	 */
  	if(leftover < max) {
  	    threshold= -max % max;
  	    while (leftover < threshold) {
@@ -162,30 +159,37 @@ math_zrand_int(UNUSED(char *name), int argc, mnumber *argv, UNUSED(int id))
  {
      mnumber ret;
      uint32_t i;
-    zlong lower=0, upper=UINT32_MAX,incl=0, diff;
+    zlong lower = 0,
+	  upper = UINT32_MAX,
+	  incl = 0,
+	  diff = UINT32_MAX;
ret.type = MN_INTEGER; switch (argc) {
-	case 0: ret.u.l=get_srandom(NULL);
-		return ret;
-		break;
+	case 0: incl = 1; break;
  	case 3: incl = (argv[2].u.l != 0)?1:0;
  	case 2: lower = argv[1].u.l;
  	case 1: upper = argv[0].u.l;
  	default: diff = upper-lower+incl;
      }
- if (lower < 0 || lower >= UINT32_MAX) {
-	zwarn("Lower bound (%z) out of range: 0-4294967295",lower);
+    if (lower < 0 || lower > UINT32_MAX) {
+	zwarn("lower bound (%z) out of range: 0-4294967295",lower);
+	ret.u.l = 0; return ret;
      } else if (upper < lower) {
-	zwarn("Upper bound (%z) must be greater than Lower Bound (%z)",upper,lower);
-    } else if (upper < 0 || upper >= UINT32_MAX) {
-	zwarn("Upper bound (%z) out of range: 0-4294967295",upper);
+	zwarn("upper bound (%z) must be greater than lower bound (%z)",upper,lower);
+	ret.u.l = 0; return ret;
+    } else if (upper < 0 || upper > UINT32_MAX) {
+	zwarn("upper bound (%z) out of range: 0-4294967295", upper);
+	ret.u.l = 0; return ret;
      }
- if ( diff == 0 ) {
+    if (diff == 0) {
  	ret.u.l=upper; /* still not convinced this shouldn't be an error. */
+    } else if (upper == UINT32_MAX && lower == 0 && incl == 1) {
+	ret.u.l = get_srandom(NULL);
+	return ret;
      } else {
  	get_bound_random_buffer(&i,1,(uint32_t) diff);
  	ret.u.l=i+lower;
@@ -253,9 +257,8 @@ setup_(UNUSED(Module m))
  	return 1;
      }
- errno=0;
      if (!(S_ISCHR(st.st_mode)) ) {
-	zwarn("Error getting kernel random pool: %e.", errno);
+	zwarn("Error getting kernel random pool: /dev/urandom is not a character device");
  	return 1;
      }
  #endif /* USE_URANDOM */
diff --git a/Src/Modules/random_real.c b/Src/Modules/random_real.c
index 4a8fcae19a..3fc279b6db 100644
--- a/Src/Modules/random_real.c
+++ b/Src/Modules/random_real.c
@@ -70,7 +70,7 @@ _zclz64(uint64_t x) {
      }
      if (!(x & 0xC000000000000000ull)) {
  	n+=2;
-	x<<=1;
+	x<<=2;
      }
      if (!(x & 0x8000000000000000ull)) {
  	n+=1;






Messages sorted by: Reverse Date, Date, Thread, Author