From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id AE8B345C304 for ; Fri, 5 Jun 2020 02:43:26 +0300 (MSK) From: Vladislav Shpilevoy Date: Fri, 5 Jun 2020 01:43:12 +0200 Message-Id: <7a8f49d2b31421f6672d5e4c6d47aff8e4d69a6a.1591313754.git.v.shpilevoy@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 05/11] xrow: don't cast double to float unconditionally List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, tsafin@tarantool.org, alyapunov@tarantool.org Xrow update functions use function xrow_update_arith_make() to execute arithmetic operation. In case any operand is a floating point value, both are cast to double. Afterwards the result value tries to shrink to float if possible, to save space. The check whether the value fits float was done in a dumb but simple way: (double)(float)value == value This was done to see if the fractional part is not truncated. The integral part was checked against FLT_MAX and -FLT_MAX. But the latter check was done after the first check. That led to undefined behaviour if the |value| is bigger than |FLT_MAX|. This patch makes xrow update firstly check the integral bounds and then the fractional part. Alongside there is fixed a bug, when FLT_MAX value was considered double. Part of #4609 --- src/box/xrow_update_field.c | 19 ++++++++++--------- test/box/update.result | 2 +- test/box/update.test.lua | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c index cc64cf955..d635e82b9 100644 --- a/src/box/xrow_update_field.c +++ b/src/box/xrow_update_field.c @@ -404,16 +404,17 @@ xrow_update_arith_make(struct xrow_update_op *op, unreachable(); break; } - float fc = (float) c; - if ((lowest_type == XUPDATE_TYPE_DOUBLE && c != (double) fc) || - (lowest_type == XUPDATE_TYPE_FLOAT && - (c >= FLT_MAX || c <= -FLT_MAX))) { - ret->type = XUPDATE_TYPE_DOUBLE; - ret->dbl = c; - } else { - ret->type = XUPDATE_TYPE_FLOAT; - ret->flt = fc; + if (c <= FLT_MAX && c >= -FLT_MAX) { + float fc = (float)c; + if (c == (double)fc) { + ret->type = XUPDATE_TYPE_FLOAT; + ret->flt = fc; + return 0; + } } + ret->type = XUPDATE_TYPE_DOUBLE; + ret->dbl = c; + return 0; } else { decimal_t a, b, c; if (! xrow_update_arg_arith_to_decimal(arg1, &a) || diff --git a/test/box/update.result b/test/box/update.result index 04866006a..6ab1a5bc5 100644 --- a/test/box/update.result +++ b/test/box/update.result @@ -1687,7 +1687,7 @@ tests = { {{'float', flt_max}, {'float', flt_max}, mp_double}, \ {{'float', -flt_max}, {'float', -flt_max}, mp_double}, \ {{'float', 1}, {'int', 1}, mp_float}, \ - {{'float', flt_max}, {'uint64_t', uint_max}, mp_double}, \ + {{'float', flt_max}, {'uint64_t', uint_max}, mp_float}, \ {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double}, \ } --- diff --git a/test/box/update.test.lua b/test/box/update.test.lua index d52be4f3b..1538cae9c 100644 --- a/test/box/update.test.lua +++ b/test/box/update.test.lua @@ -624,7 +624,7 @@ tests = { {{'float', -flt_max}, {'float', -flt_max}, mp_double}, \ -- Float + int is float when fits the float range. \ {{'float', 1}, {'int', 1}, mp_float}, \ - {{'float', flt_max}, {'uint64_t', uint_max}, mp_double}, \ + {{'float', flt_max}, {'uint64_t', uint_max}, mp_float}, \ -- Precision matters too. Double is used when need to avoid \ -- precision loss. \ {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double}, \ -- 2.21.1 (Apple Git-122.3)