[PATCH v2 2/8] decimal: fix encoding numbers with positive exponent.
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Aug 14 14:56:43 MSK 2019
On Thu, Aug 08, 2019 at 02:55:53PM +0300, Serge Petrenko wrote:
> When a number having a positive exponent is encoded, the internal
> decPackedFromNumber function returns a negative scale, which differs
> from the scale, returned by decimal_scale(). This leads to errors in
> decoding. Account for negative scale in decimal_pack() and
> decimal_unpack().
>
> Follow-up #692
> ---
> src/lib/core/decimal.c | 10 +++++++---
> test/unit/decimal.c | 15 +++++++++++++--
> test/unit/decimal.result | 11 ++++++++---
> 3 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 6ef351f81..840aa5dfe 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -33,6 +33,7 @@
> #include "third_party/decNumber/decContext.h"
> #include "third_party/decNumber/decPacked.h"
> #include "lib/core/tt_static.h"
> +#include "lib/msgpuck/msgpuck.h"
> #include <stddef.h>
> #include <stdlib.h>
> #include <float.h> /* DBL_DIG */
> @@ -340,12 +341,15 @@ char *
> decimal_pack(char *data, const decimal_t *dec)
> {
> uint32_t len = decimal_len(dec);
> - *data++ = decimal_scale(dec);
> + /* reserve space for resulting scale */
> + char *svp = data++;
> len--;
> int32_t scale;
> char *tmp = (char *)decPackedFromNumber((uint8_t *)data, len, &scale, dec);
> assert(tmp == data);
> - assert(scale == (int32_t)decimal_scale(dec));
> + /* scale may be negative, when exponent is > 0 */
> + assert(scale == (int32_t)decimal_scale(dec) || scale < 0);
This assertion now looks kinda lopsided/incomplete and therefore
confusing: we check positive scale but not negative scale. I'd rather
remove it altogether.
> + mp_store_u8(svp, (int8_t)scale);
What is the reason to use mp_store/load helpers? AFAIU plain load/store
should work just fine (as they did before this patch).
Do I understand correctly that this particular patch must be committed
to all stable branches as well?
More information about the Tarantool-patches
mailing list