Refactor BCMath bundledlib and extension (#10774)

* ext/bcmath: coding style: use indentation

And add braces to block statements, as the current code was pretty much unreadable with how inconsistent it was.

* ext/bcmath: Remove some useless header inclusions

* ext/bcmath: Use standard C99 bool type instead of char

* ext/bcmath: Include specific headers instead of config.h

* Restructure definitions to reduce header inclusions

* Use size_t as a more appropriate type

* Remove unused variable full_scale

* Refactor bc_raisemod() to get rid of Zend dependencies

This separates the concerns of throwing exceptions back into the PHP_FUNCTION instead of being the responsibility of the library

* Refactor bc_raise() to get rid of Zend dependencies

This separates the concerns of throwing exceptions back into the PHP_FUNCTION instead of being the responsibility of the library

* Refactor bc_divmod() and bc_modulo() to return bool

Return false on division by 0 attempt instead of -1 and true on success instead of 0

* Refactor bc_divide() to return bool

Return false on division by 0 attempt instead of -1 and true on success instead of 0

Author: Girgias

ext/bcmath/bcmath.c

@@ -32,6 +32,9 @@
 ZEND_DECLARE_MODULE_GLOBALS(bcmath)
 static PHP_GINIT_FUNCTION(bcmath);
 static PHP_GSHUTDOWN_FUNCTION(bcmath);
+static PHP_MINIT_FUNCTION(bcmath);
+static PHP_MSHUTDOWN_FUNCTION(bcmath);
+static PHP_MINFO_FUNCTION(bcmath);
 
 zend_module_entry bcmath_module_entry = {
 	STANDARD_MODULE_HEADER,
@@ -341,15 +344,13 @@ PHP_FUNCTION(bcdiv)
 		goto cleanup;
 	}
 
-	switch (bc_divide(first, second, &result, scale)) {
-		case 0: /* OK */
-			RETVAL_STR(bc_num2str_ex(result, scale));
-			break;
-		case -1: /* division by zero */
-			zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Division by zero");
-			break;
+	if (!bc_divide(first, second, &result, scale)) {
+		zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Division by zero");
+		goto cleanup;
 	}
 
+	RETVAL_STR(bc_num2str_ex(result, scale));
+
 	cleanup: {
 		bc_free_num(&first);
 		bc_free_num(&second);
@@ -397,15 +398,13 @@ PHP_FUNCTION(bcmod)
 		goto cleanup;
 	}
 
-	switch (bc_modulo(first, second, &result, scale)) {
-		case 0:
-			RETVAL_STR(bc_num2str_ex(result, scale));
-			break;
-		case -1:
-			zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Modulo by zero");
-			break;
+	if (!bc_modulo(first, second, &result, scale)) {
+		zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Modulo by zero");
+		goto cleanup;
 	}
 
+	RETVAL_STR(bc_num2str_ex(result, scale));
+
 	cleanup: {
 		bc_free_num(&first);
 		bc_free_num(&second);
@@ -417,16 +416,16 @@ PHP_FUNCTION(bcmod)
 /* {{{ Returns the value of an arbitrary precision number raised to the power of another reduced by a modulus */
 PHP_FUNCTION(bcpowmod)
 {
-	zend_string *left, *right, *modulus;
+	zend_string *base_str, *exponent_str, *modulus_str;
 	zend_long scale_param;
 	bool scale_param_is_null = 1;
-	bc_num first, second, mod, result;
+	bc_num bc_base, bc_expo, bc_modulus, result;
 	int scale = BCG(bc_precision);
 
 	ZEND_PARSE_PARAMETERS_START(3, 4)
-		Z_PARAM_STR(left)
-		Z_PARAM_STR(right)
-		Z_PARAM_STR(modulus)
+		Z_PARAM_STR(base_str)
+		Z_PARAM_STR(exponent_str)
+		Z_PARAM_STR(modulus_str)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_LONG_OR_NULL(scale_param, scale_param_is_null)
 	ZEND_PARSE_PARAMETERS_END();
@@ -440,34 +439,53 @@ PHP_FUNCTION(bcpowmod)
 		scale = (int) scale_param;
 	}
 
-	bc_init_num(&first);
-	bc_init_num(&second);
-	bc_init_num(&mod);
+	bc_init_num(&bc_base);
+	bc_init_num(&bc_expo);
+	bc_init_num(&bc_modulus);
 	bc_init_num(&result);
 
-	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+	if (php_str2num(&bc_base, ZSTR_VAL(base_str)) == FAILURE) {
 		zend_argument_value_error(1, "is not well-formed");
 		goto cleanup;
 	}
 
-	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+	if (php_str2num(&bc_expo, ZSTR_VAL(exponent_str)) == FAILURE) {
 		zend_argument_value_error(2, "is not well-formed");
 		goto cleanup;
 	}
 
-	if (php_str2num(&mod, ZSTR_VAL(modulus)) == FAILURE) {
+	if (php_str2num(&bc_modulus, ZSTR_VAL(modulus_str)) == FAILURE) {
 		zend_argument_value_error(3, "is not well-formed");
 		goto cleanup;
 	}
 
-	if (bc_raisemod(first, second, mod, &result, scale) == SUCCESS) {
-		RETVAL_STR(bc_num2str_ex(result, scale));
+	raise_mod_status status = bc_raisemod(bc_base, bc_expo, bc_modulus, &result, scale);
+	switch (status) {
+		case BASE_HAS_FRACTIONAL:
+			zend_argument_value_error(1, "cannot have a fractional part");
+			goto cleanup;
+		case EXPO_HAS_FRACTIONAL:
+			zend_argument_value_error(2, "cannot have a fractional part");
+			goto cleanup;
+		case EXPO_IS_NEGATIVE:
+			zend_argument_value_error(2, "must be greater than or equal to 0");
+			goto cleanup;
+		case MOD_HAS_FRACTIONAL:
+			zend_argument_value_error(3, "cannot have a fractional part");
+			goto cleanup;
+		case MOD_IS_ZERO:
+			zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Modulo by zero");
+			goto cleanup;
+		case OK:
+			RETVAL_STR(bc_num2str_ex(result, scale));
+			break;
+		EMPTY_SWITCH_DEFAULT_CASE();
 	}
 
 	cleanup: {
-		bc_free_num(&first);
-		bc_free_num(&second);
-		bc_free_num(&mod);
+		bc_free_num(&bc_base);
+		bc_free_num(&bc_expo);
+		bc_free_num(&bc_modulus);
 		bc_free_num(&result);
 	};
 }
@@ -476,15 +494,15 @@ PHP_FUNCTION(bcpowmod)
 /* {{{ Returns the value of an arbitrary precision number raised to the power of another */
 PHP_FUNCTION(bcpow)
 {
-	zend_string *left, *right;
+	zend_string *base_str, *exponent_str;
 	zend_long scale_param;
 	bool scale_param_is_null = 1;
-	bc_num first, second, result;
+	bc_num first, bc_exponent, result;
 	int scale = BCG(bc_precision);
 
 	ZEND_PARSE_PARAMETERS_START(2, 3)
-		Z_PARAM_STR(left)
-		Z_PARAM_STR(right)
+		Z_PARAM_STR(base_str)
+		Z_PARAM_STR(exponent_str)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_LONG_OR_NULL(scale_param, scale_param_is_null)
 	ZEND_PARSE_PARAMETERS_END();
@@ -499,26 +517,37 @@ PHP_FUNCTION(bcpow)
 	}
 
 	bc_init_num(&first);
-	bc_init_num(&second);
+	bc_init_num(&bc_exponent);
 	bc_init_num(&result);
 
-	if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
+	if (php_str2num(&first, ZSTR_VAL(base_str)) == FAILURE) {
 		zend_argument_value_error(1, "is not well-formed");
 		goto cleanup;
 	}
 
-	if (php_str2num(&second, ZSTR_VAL(right)) == FAILURE) {
+	if (php_str2num(&bc_exponent, ZSTR_VAL(exponent_str)) == FAILURE) {
 		zend_argument_value_error(2, "is not well-formed");
 		goto cleanup;
 	}
 
-	bc_raise (first, second, &result, scale);
+	/* Check the exponent for scale digits and convert to a long. */
+	if (bc_exponent->n_scale != 0) {
+		zend_argument_value_error(2, "cannot have a fractional part");
+		goto cleanup;
+	}
+	long exponent = bc_num2long(bc_exponent);
+	if (exponent == 0 && (bc_exponent->n_len > 1 || bc_exponent->n_value[0] != 0)) {
+		zend_argument_value_error(2, "is too large");
+		goto cleanup;
+	}
+
+	bc_raise(first, exponent, &result, scale);
 
 	RETVAL_STR(bc_num2str_ex(result, scale));
 
 	cleanup: {
 		bc_free_num(&first);
-		bc_free_num(&second);
+		bc_free_num(&bc_exponent);
 		bc_free_num(&result);
 	};
 }

ext/bcmath/libbcmath/src/add.c

@@ -29,55 +29,49 @@
 
 *************************************************************************/
 
-#include <config.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <ctype.h>
-#include <stdarg.h>
 #include "bcmath.h"
 #include "private.h"
+#include <stddef.h>
+#include <string.h>
 
 
 /* Here is the full add routine that takes care of negative numbers.
    N1 is added to N2 and the result placed into RESULT.  SCALE_MIN
    is the minimum scale for the result. */
 
-void bc_add (bc_num n1, bc_num n2, bc_num *result, int scale_min)
+void bc_add (bc_num n1, bc_num n2, bc_num *result, size_t scale_min)
 {
-  bc_num sum = NULL;
-  int cmp_res;
-  int res_scale;
+	bc_num sum = NULL;
+	int cmp_res;
+	size_t res_scale;
 
-  if (n1->n_sign == n2->n_sign)
-    {
-      sum = _bc_do_add (n1, n2, scale_min);
-      sum->n_sign = n1->n_sign;
-    }
-  else
-    {
-      /* subtraction must be done. */
-      cmp_res = _bc_do_compare (n1, n2, FALSE, FALSE);  /* Compare magnitudes. */
-      switch (cmp_res)
-	{
-	case -1:
-	  /* n1 is less than n2, subtract n1 from n2. */
-	  sum = _bc_do_sub (n2, n1, scale_min);
-	  sum->n_sign = n2->n_sign;
-	  break;
-	case  0:
-	  /* They are equal! return zero with the correct scale! */
-	  res_scale = MAX (scale_min, MAX(n1->n_scale, n2->n_scale));
-	  sum = bc_new_num (1, res_scale);
-	  memset (sum->n_value, 0, res_scale+1);
-	  break;
-	case  1:
-	  /* n2 is less than n1, subtract n2 from n1. */
-	  sum = _bc_do_sub (n1, n2, scale_min);
-	  sum->n_sign = n1->n_sign;
+	if (n1->n_sign == n2->n_sign) {
+		sum = _bc_do_add (n1, n2, scale_min);
+		sum->n_sign = n1->n_sign;
+	} else {
+		/* subtraction must be done. */
+		/* Compare magnitudes. */
+		cmp_res = _bc_do_compare(n1, n2, false, false);
+		switch (cmp_res) {
+			case -1:
+				/* n1 is less than n2, subtract n1 from n2. */
+				sum = _bc_do_sub (n2, n1, scale_min);
+				sum->n_sign = n2->n_sign;
+				break;
+			case  0:
+				/* They are equal! return zero with the correct scale! */
+				res_scale = MAX (scale_min, MAX(n1->n_scale, n2->n_scale));
+				sum = bc_new_num (1, res_scale);
+				memset (sum->n_value, 0, res_scale+1);
+				break;
+			case  1:
+				/* n2 is less than n1, subtract n2 from n1. */
+				sum = _bc_do_sub (n1, n2, scale_min);
+				sum->n_sign = n1->n_sign;
+		}
 	}
-    }
 
-  /* Clean up and return. */
-  bc_free_num (result);
-  *result = sum;
+	/* Clean up and return. */
+	bc_free_num (result);
+	*result = sum;
 }