From: Serge Petrenko <sergepetrenko@tarantool.org>
To: vdavydov.dev@gmail.com
Cc: tarantool-patches@freelists.org,
Serge Petrenko <sergepetrenko@tarantool.org>
Subject: [PATCH v3 2/5] decimal: diallow infinity and NaN entirely.
Date: Tue, 2 Jul 2019 20:27:49 +0300 [thread overview]
Message-ID: <6060795d4948ab34c474bdba45c3afdebbfc35c7.1562087728.git.sergepetrenko@tarantool.org> (raw)
In-Reply-To: <cover.1562087728.git.sergepetrenko@tarantool.org>
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)
next prev parent reply other threads:[~2019-07-02 17:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-02 17:27 [PATCH v3 0/5] decimal: expose decimal module to Lua Serge Petrenko
2019-07-02 17:27 ` [PATCH v3 1/5] decimal: fix ln hang on values between ~ 0.9 and 1.1 Serge Petrenko
2019-07-02 17:27 ` Serge Petrenko [this message]
2019-07-02 17:27 ` [PATCH v3 3/5] decimal: fix string formatting on construction from double Serge Petrenko
2019-07-02 17:27 ` [PATCH v3 4/5] lua/utils: add a function to register FFI metatypes Serge Petrenko
2019-07-02 17:27 ` [PATCH v3 5/5] decimal: expose decimal type to lua Serge Petrenko
2019-07-03 6:50 ` [tarantool-patches] " Serge Petrenko
2019-07-08 14:25 ` [PATCH v3 0/5] decimal: expose decimal module to Lua Vladimir Davydov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6060795d4948ab34c474bdba45c3afdebbfc35c7.1562087728.git.sergepetrenko@tarantool.org \
--to=sergepetrenko@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=vdavydov.dev@gmail.com \
--subject='Re: [PATCH v3 2/5] decimal: diallow infinity and NaN entirely.' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox