[PATCH v3 2/5] decimal: diallow infinity and NaN entirely.

Serge Petrenko sergepetrenko at tarantool.org
Tue Jul 2 20:27:49 MSK 2019


While arithmetic operations do not return infinities or NaNs, it is
possbile to construct an invalid decimal value from strings 'Infinity',
'NaN' and similar. Some decimal mathematic functions may also result in
an infinity, say, ln(0) yields '-Infinity'.
So, add checks that the number is not a NaN or infinity after each
operation, so that the operation either returns an error, or a valid
finite decimal number.

Follow-up 6d62c6c19ed418932ead1bba44fcd7cd84c78a19
---
 src/lib/core/decimal.c   | 73 +++++++++++-----------------------------
 test/unit/decimal.c      | 18 +++++++++-
 test/unit/decimal.result | 39 ++++++++++++++-------
 3 files changed, 62 insertions(+), 68 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index c76387d76..91ee3f307 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -69,13 +69,14 @@ static __thread decContext decimal_context = {
 
 /**
  * A finalizer for all the operations.
- * Check the operation context status and empty it.
+ * Check the operation context status and empty it,
+ * check that result isn't a NaN or infinity.
  *
  * @return NULL if finalization failed.
  *         result pointer otherwise.
  */
-static inline int
-decimal_check_status(decContext *context)
+static decimal_t *
+decimal_check_status(decimal_t *dec, decContext *context)
 {
 	uint32_t status = decContextGetStatus(context);
 	decContextZeroStatus(context);
@@ -88,7 +89,7 @@ decimal_check_status(decContext *context)
 	 */
 	status &= ~(uint32_t)(DEC_Inexact | DEC_Rounded | DEC_Underflow |
 			      DEC_Subnormal | DEC_Clamped);
-	return status ? -1 : 0;
+	return status || !decNumberIsFinite(dec) ? NULL : dec;
 }
 
 int decimal_precision(const decimal_t *dec) {
@@ -111,11 +112,7 @@ decimal_t *
 decimal_from_string(decimal_t *dec, const char *str)
 {
 	decNumberFromString(dec, str, &decimal_context);
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return dec;
-	}
+	return decimal_check_status(dec, &decimal_context);
 }
 
 decimal_t *
@@ -157,7 +154,7 @@ decimal_compare(const decimal_t *lhs, const decimal_t *rhs)
 	decNumber res;
 	decNumberCompare(&res, lhs, rhs, &decimal_context);
 	int r = decNumberToInt32(&res, &decimal_context);
-	assert(decimal_check_status(&decimal_context) == 0);
+	assert(decimal_check_status(&res, &decimal_context) != NULL);
 	return r;
 }
 
@@ -182,7 +179,7 @@ decimal_round(decimal_t *dec, int scale)
 	};
 
 	decNumberPlus(dec, dec, &context);
-	assert(decimal_check_status(&context) == 0);
+	assert(decimal_check_status(dec, &context) != NULL);
 	return dec;
 }
 
@@ -190,7 +187,7 @@ decimal_t *
 decimal_abs(decimal_t *res, const decimal_t *dec)
 {
 	decNumberAbs(res, dec, &decimal_context);
-	assert(decimal_check_status(&decimal_context) == 0);
+	assert(decimal_check_status(res, &decimal_context) != NULL);
 	return res;
 }
 
@@ -198,7 +195,7 @@ decimal_t *
 decimal_minus(decimal_t *res, const decimal_t *dec)
 {
 	decNumberMinus(res, dec, &decimal_context);
-	assert(decimal_check_status(&decimal_context) == 0);
+	assert(decimal_check_status(res, &decimal_context) != NULL);
 	return res;
 }
 
@@ -206,11 +203,7 @@ decimal_t *
 decimal_add(decimal_t *res, const decimal_t *lhs, const decimal_t *rhs)
 {
 	decNumberAdd(res, lhs, rhs, &decimal_context);
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -218,11 +211,7 @@ decimal_sub(decimal_t *res, const decimal_t *lhs, const decimal_t *rhs)
 {
 	decNumberSubtract(res, lhs, rhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -230,11 +219,7 @@ decimal_mul(decimal_t *res, const decimal_t *lhs, const decimal_t *rhs)
 {
 	decNumberMultiply(res, lhs, rhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -242,11 +227,7 @@ decimal_div(decimal_t *res, const decimal_t *lhs, const decimal_t *rhs)
 {
 	decNumberDivide(res, lhs, rhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -254,11 +235,7 @@ decimal_log10(decimal_t *res, const decimal_t *lhs)
 {
 	decNumberLog10(res, lhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -279,7 +256,7 @@ decimal_ln(decimal_t *res, const decimal_t *lhs)
 	decNumberLn(res, lhs, &decimal_context);
 
 	decimal_context.emin = emin;
-	if (decimal_check_status(&decimal_context) != 0) {
+	if (decimal_check_status(res, &decimal_context) == NULL) {
 		return NULL;
 	} else {
 		/*
@@ -297,11 +274,7 @@ decimal_pow(decimal_t *res, const decimal_t *lhs, const decimal_t *rhs)
 {
 	decNumberPower(res, lhs, rhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -309,11 +282,7 @@ decimal_exp(decimal_t *res, const decimal_t *lhs)
 {
 	decNumberExp(res, lhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 decimal_t *
@@ -321,11 +290,7 @@ decimal_sqrt(decimal_t *res, const decimal_t *lhs)
 {
 	decNumberSquareRoot(res, lhs, &decimal_context);
 
-	if (decimal_check_status(&decimal_context) != 0) {
-		return NULL;
-	} else {
-		return res;
-	}
+	return decimal_check_status(res, &decimal_context);
 }
 
 uint32_t
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index 327a06d72..ae806a106 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -62,6 +62,12 @@
 	is(decimal_##op(&c, &a, &b), NULL, "decimal_"#op"("#stra", "#strb") - overflow");\
 })
 
+#define dectest_op1_fail(op, stra) ({\
+	decimal_t a, b;\
+	is(decimal_from_string(&a, #stra), &a, "decimal_from_string("#stra")");\
+	is(decimal_##op(&b, &a), NULL, "decimal_"#op"("#stra") - error on wrong operands.");\
+})
+
 char buf[32];
 
 #define test_decpack(str) ({\
@@ -121,7 +127,7 @@ test_pack_unpack(void)
 int
 main(void)
 {
-	plan(266);
+	plan(279);
 
 	dectest(314, 271, uint64, uint64_t);
 	dectest(65535, 23456, uint64, uint64_t);
@@ -162,6 +168,10 @@ main(void)
 	dectest_construct(double, 2e38, failure);
 	dectest_construct(string, "1e38", failure);
 	dectest_construct(string, "100000000000000000000000000000000000000", failure);
+	/* Check that inf and NaN are not allowed. Check bad input. */
+	dectest_construct(string, "inf", failure);
+	dectest_construct(string, "NaN", failure);
+	dectest_construct(string, "a random string", failure);
 
 	dectest_construct(int64, LONG_MIN, success);
 	dectest_construct(int64, LONG_MAX, success);
@@ -171,6 +181,12 @@ main(void)
 	dectest_op_fail(mul, 1e19, 1e19);
 	dectest_op_fail(div, 1e19, 1e-19);
 
+	dectest_op1_fail(ln, 0);
+	dectest_op1_fail(ln, -1);
+	dectest_op1_fail(log10, 0);
+	dectest_op1_fail(log10, -1);
+	dectest_op1_fail(sqrt, -10);
+
 	test_pack_unpack();
 
 	return check_plan();
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index 7e767dcd2..1c72cdfab 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -1,4 +1,4 @@
-1..266
+1..279
 ok 1 - decimal(314)
 ok 2 - decimal(271)
 ok 3 - decimal(314) + decimal(271)
@@ -252,18 +252,31 @@ ok 250 - decimal_compare(10)
 ok 251 - decimal construction from 2e38 failure
 ok 252 - decimal construction from "1e38" failure
 ok 253 - decimal construction from "100000000000000000000000000000000000000" failure
-ok 254 - decimal construction from LONG_MIN success
-ok 255 - decimal construction from LONG_MAX success
-ok 256 - decimal construction from ULONG_MAX success
-ok 257 - decimal_from_string(9e37)
-ok 258 - decimal_from_string(1e37)
-ok 259 - decimal_add(9e37, 1e37) - overflow
-ok 260 - decimal_from_string(1e19)
-ok 261 - decimal_from_string(1e19)
-ok 262 - decimal_mul(1e19, 1e19) - overflow
+ok 254 - decimal construction from "inf" failure
+ok 255 - decimal construction from "NaN" failure
+ok 256 - decimal construction from "a random string" failure
+ok 257 - decimal construction from LONG_MIN success
+ok 258 - decimal construction from LONG_MAX success
+ok 259 - decimal construction from ULONG_MAX success
+ok 260 - decimal_from_string(9e37)
+ok 261 - decimal_from_string(1e37)
+ok 262 - decimal_add(9e37, 1e37) - overflow
 ok 263 - decimal_from_string(1e19)
-ok 264 - decimal_from_string(1e-19)
-ok 265 - decimal_div(1e19, 1e-19) - overflow
+ok 264 - decimal_from_string(1e19)
+ok 265 - decimal_mul(1e19, 1e19) - overflow
+ok 266 - decimal_from_string(1e19)
+ok 267 - decimal_from_string(1e-19)
+ok 268 - decimal_div(1e19, 1e-19) - overflow
+ok 269 - decimal_from_string(0)
+ok 270 - decimal_ln(0) - error on wrong operands.
+ok 271 - decimal_from_string(-1)
+ok 272 - decimal_ln(-1) - error on wrong operands.
+ok 273 - decimal_from_string(0)
+ok 274 - decimal_log10(0) - error on wrong operands.
+ok 275 - decimal_from_string(-1)
+ok 276 - decimal_log10(-1) - error on wrong operands.
+ok 277 - decimal_from_string(-10)
+ok 278 - decimal_sqrt(-10) - error on wrong operands.
     1..146
     ok 1 - decimal_len(0)
     ok 2 - decimal_len(0) == len(decimal_pack(0)
@@ -411,4 +424,4 @@ ok 265 - decimal_div(1e19, 1e-19) - overflow
     ok 144 - str(decimal_unpack(decimal_pack(-99999999999999999999999999999999999999)) == -99999999999999999999999999999999999999
     ok 145 - unpack malformed decimal fails
     ok 146 - decode malformed decimal preserves buffer position
-ok 266 - subtests
+ok 279 - subtests
-- 
2.20.1 (Apple Git-117)




More information about the Tarantool-patches mailing list