[Freeciv-Dev] Re: (PR#2476) myrand does the wrong thing

["Raimar Falke via RT" <rt@xxxxxxxxxxxxxx>]

On Mon, Dec 02, 2002 at 02:54:18PM -0800, Guest via RT wrote:
> 
> I was testing what happens on very small maps, and I found that the
> system broke consistently with a ysize less than 18. A bit of gdb, and
> the problem is that myrand is being called with a value of zero, which
> returns a random number between zero and MAX_UINT32. If we're looking
> for a very small random number, getting a very large random number will
> produce problems.
> 
> This patch makes myrand(size) return zero immediately if its argument is
> less than or equal to 1. Additionally, it fixes the only place where
> myrand(0) is explicitely called (in client initialization), so now it
> calls myrand(MAX_UINT32). If you want a random number between zero and
> MAX_UINT32, why not ask for one?

Problems: the call myrand(0) wouldn't change internal state. This is
bad. The attached patch fixes this by always going trough the
next-state step and has no early return. It also comment the function
a bit (now that I understood it).

Please test.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  reality.sys corrupt. Reboot Universe? (y,n,q)

Index: common/rand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/rand.c,v
retrieving revision 1.8
diff -u -r1.8 rand.c
--- common/rand.c       2002/11/14 09:15:03     1.8
+++ common/rand.c       2002/12/03 19:19:18
@@ -53,7 +53,7 @@
 /*************************************************************************
   Returns a new random value from the sequence, in the interval 0 to
   (size-1) inclusive, and updates global state for next call.
-  size==0 means result will be within 0 to MAX_UINT32 inclusive.
+  This means that if size <= 1 the function will always return 0.
 
   Once we calculate new_rand below uniform (we hope) between 0 and
   MAX_UINT32 inclusive, need to reduce to required range.  Using
@@ -79,14 +79,23 @@
 *************************************************************************/
 RANDOM_TYPE myrand(RANDOM_TYPE size) 
 {
-  RANDOM_TYPE new_rand, divisor=1, max=MAX_UINT32;
+  RANDOM_TYPE new_rand, divisor, max;
   int bailout = 0;
 
   assert(rand_state.is_init);
     
-  if (size>1) {
-    divisor = MAX_UINT32/size;
+  if (size > 1) {
+    divisor = MAX_UINT32 / size;
     max = size * divisor - 1;
+  } else {
+    /* size == 0 || size == 1 */
+
+    /* 
+     * These assignments are only here to make the compiler
+     * happy. Since each usage is guarded with a if(size>1).
+     */
+    max = MAX_UINT32;
+    divisor = 1;
   }
 
   do {
@@ -104,14 +113,13 @@
       break;
     }
 
-  } while (new_rand > max && size > 1);
+  } while (size > 1 && new_rand > max);
 
   if (size > 1) {
     new_rand /= divisor;
-  } else if (size == 1) {
+  } else {
     new_rand = 0;
   }
-  /* else leave it "raw" */
 
   /* freelog(LOG_DEBUG, "rand(%u) = %u", size, new_rand); */
 
@@ -145,7 +153,7 @@
      * problems even using divisor.
      */
     for (i=0; i<10000; i++) {
-      (void) myrand(0);
+      (void) myrand(MAX_UINT32);
     }
 }