Address issues raised by @nikic

 - Thread safety on rand init function.
 - Ret false on validation failure
 - Add _dep of temp_a to temp_b
 - Special case int sized min values
 - More tests!

3 files changed, 141 insertions, 22 deletions

diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c
index 2a6cc3d3c0..8aff2d6b23 100644
--- a/ext/gmp/gmp.c
+++ b/ext/gmp/gmp.c
@@ -1795,7 +1795,7 @@ ZEND_FUNCTION(gmp_sign)
 }
 /* }}} */
 
-void gmp_init_random()
+static void gmp_init_random(TSRMLS_D)
 {
 	if (!GMPG(rand_initialized)) {
 		/* Initialize */
@@ -1823,7 +1823,7 @@ ZEND_FUNCTION(gmp_random)
 	}
 
 	INIT_GMP_RETVAL(gmpnum_result);
-	gmp_init_random();
+	gmp_init_random(TSRMLS_C);
 
 #ifdef GMP_LIMB_BITS
 	mpz_urandomb(gmpnum_result, GMPG(rand_state), GMP_ABS (limiter) * GMP_LIMB_BITS);
@@ -1846,11 +1846,11 @@ ZEND_FUNCTION(gmp_random_bits)
 
 	if (bits <= 0) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "The number of bits must be positive");
-		return;
+		RETURN_FALSE;
 	}
 
 	INIT_GMP_RETVAL(gmpnum_result);
-	gmp_init_random();
+	gmp_init_random(TSRMLS_C);
 
 	mpz_urandomb(gmpnum_result, GMPG(rand_state), bits);
 }
@@ -1868,26 +1868,53 @@ ZEND_FUNCTION(gmp_random_range)
 		return;
 	}
 
-	FETCH_GMP_ZVAL(gmpnum_min, min_arg, temp_a);
-	FETCH_GMP_ZVAL(gmpnum_max, max_arg, temp_b);
+	gmp_init_random(TSRMLS_C);
+
+	FETCH_GMP_ZVAL(gmpnum_max, max_arg, temp_a);
+
+	if (Z_TYPE_P(min_arg) == IS_LONG && Z_LVAL_P(min_arg) >= 0) {
+		if (mpz_cmp_ui(gmpnum_max, Z_LVAL_P(min_arg)) <= 0) {
+			FREE_GMP_TEMP(temp_a);
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "The minimum value must be less than the maximum value");
+			RETURN_FALSE;
+		}
+
+		INIT_GMP_RETVAL(gmpnum_result);
+
+		if (Z_LVAL_P(min_arg)) {
+			mpz_sub_ui(gmpnum_max, gmpnum_max, Z_LVAL_P(min_arg));
+		}
+
+		mpz_add_ui(gmpnum_max, gmpnum_max, 1);
+		mpz_urandomm(gmpnum_result, GMPG(rand_state), gmpnum_max);
+
+		if (Z_LVAL_P(min_arg)) {
+			mpz_add_ui(gmpnum_result, gmpnum_result, Z_LVAL_P(min_arg));
+		}
 
-	if (mpz_cmp(gmpnum_min, gmpnum_max) > 0) {
 		FREE_GMP_TEMP(temp_a);
-		FREE_GMP_TEMP(temp_b);
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "The minimum value must be lower than the maximum value");
-		return;	
+
 	}
+	else {
+		FETCH_GMP_ZVAL_DEP(gmpnum_min, min_arg, temp_b, temp_a);
 
-	INIT_GMP_RETVAL(gmpnum_result);
-	gmp_init_random();
+		if (mpz_cmp(gmpnum_max, gmpnum_min) <= 0) {
+			FREE_GMP_TEMP(temp_b);
+			FREE_GMP_TEMP(temp_a);
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "The minimum value must be less than the maximum value");
+			RETURN_FALSE;
+		}
 
-	mpz_sub(gmpnum_max, gmpnum_max, gmpnum_min);
-	mpz_add_ui(gmpnum_max, gmpnum_max, 1);
-	mpz_urandomm(gmpnum_result, GMPG(rand_state), gmpnum_max);
-	mpz_add(gmpnum_result, gmpnum_result, gmpnum_min);
+		INIT_GMP_RETVAL(gmpnum_result);
 
-	FREE_GMP_TEMP(temp_a);
-	FREE_GMP_TEMP(temp_b);
+		mpz_sub(gmpnum_max, gmpnum_max, gmpnum_min);
+		mpz_add_ui(gmpnum_max, gmpnum_max, 1);
+		mpz_urandomm(gmpnum_result, GMPG(rand_state), gmpnum_max);
+		mpz_add(gmpnum_result, gmpnum_result, gmpnum_min);
+
+		FREE_GMP_TEMP(temp_b);
+		FREE_GMP_TEMP(temp_a);
+	}
 }
 /* }}} */
 
diff --git a/ext/gmp/tests/gmp_random_bits.phpt b/ext/gmp/tests/gmp_random_bits.phpt
index 9046177e38..21d493cdb6 100644
--- a/ext/gmp/tests/gmp_random_bits.phpt
+++ b/ext/gmp/tests/gmp_random_bits.phpt
@@ -5,15 +5,41 @@ gmp_random_bits() basic tests
 --FILE--
 <?php
 
+var_dump(gmp_random_bits());
 var_dump(gmp_random_bits(0));
 var_dump(gmp_random_bits(-1));
 
+// If these error the test fails.
+gmp_random_bits(1);
+gmp_random_bits(1024);
+
+// 2 seconds to make sure the numbers stay in range
+$start = microtime(true);
+$limit = (2 ** 30) - 1;
+while (1) {
+	for ($i = 0; $i < 5000; $i++) {
+		$result = gmp_random_bits(30);
+		if ($result < 0 || $result > $limit) {
+			print "RANGE VIOLATION\n";
+			var_dump($result);
+			break 2;
+		}
+	}
+
+	if (microtime(true) - $start > 2) {
+		break;
+	}
+}
+
 echo "Done\n";
 ?>
 --EXPECTF--
-Warning: gmp_random_bits(): The number of bits must be positive in %s on line %d
+Warning: gmp_random_bits() expects exactly 1 parameter, 0 given in %s on line %d
 NULL
 
 Warning: gmp_random_bits(): The number of bits must be positive in %s on line %d
-NULL
+bool(false)
+
+Warning: gmp_random_bits(): The number of bits must be positive in %s on line %d
+bool(false)
 Done
diff --git a/ext/gmp/tests/gmp_random_range.phpt b/ext/gmp/tests/gmp_random_range.phpt
index 50afafd4e9..a8e7c4a9c1 100644
--- a/ext/gmp/tests/gmp_random_range.phpt
+++ b/ext/gmp/tests/gmp_random_range.phpt
@@ -5,11 +5,77 @@ gmp_random_range() basic tests
 --FILE--
 <?php
 
-var_dump(gmp_random_range(1, -1));
+$minusTen = gmp_init(-1);
+$plusTen = gmp_init(1);
+$zero = gmp_init(0);
+
+var_dump(gmp_random_range());
+var_dump(gmp_random_range(10));
+var_dump(gmp_random_range(10, -10));
+
+var_dump(gmp_random_range($plusTen, $minusTen));
+var_dump(gmp_random_range($plusTen, $zero));
+
+// If these error the test fails.
+gmp_random_range(0, 10);
+gmp_random_range(1, 10);
+gmp_random_range(-1, 10);
+gmp_random_range(-10, 0);
+gmp_random_range(-10, -1);
+
+gmp_random_range(0, $plusTen);
+gmp_random_range(1, $plusTen);
+gmp_random_range(-1, $plusTen);
+
+gmp_random_range($zero, $plusTen);
+gmp_random_range($minusTen, $plusTen);
+
+// 2 seconds to make sure the numbers stay in range
+$start = microtime(true);
+while (1) {
+	for ($i = 0; $i < 5000; $i++) {
+		$result = gmp_random_range(0, 1000);
+		if ($result < 0 || $result > 1000) {
+			print "RANGE VIOLATION 1\n";
+			var_dump($result);
+			break 2;
+		}
+
+		$result = gmp_random_range(-1000, 0);
+		if ($result < -1000 || $result > 0) {
+			print "RANGE VIOLATION 2\n";
+			var_dump($result);
+			break 2;
+		}
+
+		$result = gmp_random_range(-500, 500);
+		if ($result < -500 || $result > 500) {
+			print "RANGE VIOLATION 3\n";
+			var_dump($result);
+			break 2;
+		}
+	}
+
+	if (microtime(true) - $start > 2) {
+		break;
+	}
+}
 
 echo "Done\n";
 ?>
 --EXPECTF--
-Warning: gmp_random_range(): The minimum value must be lower than the maximum value in %s on line %d
+Warning: gmp_random_range() expects exactly 2 parameters, 0 given in %s on line %d
+NULL
+
+Warning: gmp_random_range() expects exactly 2 parameters, 1 given in %s on line %d
 NULL
+
+Warning: gmp_random_range(): The minimum value must be less than the maximum value in %s on line %d
+bool(false)
+
+Warning: gmp_random_range(): The minimum value must be less than the maximum value in %s on line %d
+bool(false)
+
+Warning: gmp_random_range(): The minimum value must be less than the maximum value in %s on line %d
+bool(false)
 Done