From 20d08db64ea2f934a55a40d9a44c78e67d73d592 Mon Sep 17 00:00:00 2001 From: Nikita Pettik <korablev@tarantool.org> Date: Tue, 23 Jul 2019 16:35:43 +0300 Subject: [PATCH] sql: always erase numeric flag after stringifying Function which converts values to string representation (sqlVdbeMemStringify()) erase MEM_Int/MEM_Real/MEM_Bool flags only when it is specified by 'force' parameter. Hence, when 'force' argument is false, memory cell after conversion will contain string value, but flag indicating its type will be equal to combination of MEM_Str and one of mentioned flags. It seems to be remains of affinity routines, since in current state memory cell must have only one type. What is more, it can lead to unpredicted consequences, for instance assertion fault (sql_value_type() assumes that value has one specific type). Let's fix it removing 'force' argument from sqlVdbeMemStringify() and always clean up type flag. --- src/box/sql/vdbe.c | 8 ++++---- src/box/sql/vdbeInt.h | 2 +- src/box/sql/vdbemem.c | 10 ++++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index f6b0366e24..3a8dc04c11 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -183,7 +183,7 @@ vdbeTakeBranch(int iSrcLine, u8 I, u8 M) * already. Return non-zero if a malloc() fails. */ #define Stringify(P) \ - if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P,0)) \ + if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P)) \ { goto no_mem; } /* @@ -339,7 +339,7 @@ mem_apply_type(struct Mem *record, enum field_type type) */ if ((record->flags & MEM_Str) == 0 && (record->flags & (MEM_Real | MEM_Int | MEM_UInt)) != 0) - sqlVdbeMemStringify(record, 1); + sqlVdbeMemStringify(record); record->flags &= ~(MEM_Real | MEM_Int | MEM_UInt); return 0; case FIELD_TYPE_SCALAR: @@ -2175,7 +2175,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ (flags1 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { testcase( pIn1->flags & MEM_Int); testcase( pIn1->flags & MEM_Real); - sqlVdbeMemStringify(pIn1, 1); + sqlVdbeMemStringify(pIn1); testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn)); flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask); assert(pIn1!=pIn3); @@ -2184,7 +2184,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ (flags3 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { testcase( pIn3->flags & MEM_Int); testcase( pIn3->flags & MEM_Real); - sqlVdbeMemStringify(pIn3, 1); + sqlVdbeMemStringify(pIn3); testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn)); flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask); } diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index 22172fbaf8..0bdcb9d308 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -493,7 +493,7 @@ void sqlVdbeMemInit(Mem *, sql *, u32); void sqlVdbeMemSetNull(Mem *); void sqlVdbeMemSetZeroBlob(Mem *, int); int sqlVdbeMemMakeWriteable(Mem *); -int sqlVdbeMemStringify(Mem *, u8); +int sqlVdbeMemStringify(Mem *); int sqlVdbeIntValue(Mem *, int64_t *, bool *is_neg); int sqlVdbeMemIntegerify(Mem *, bool is_forced); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 847a6b0cec..268b5e9798 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -277,7 +277,7 @@ sqlVdbeMemNulTerminate(Mem * pMem) * user and the latter is an internal programming error. */ int -sqlVdbeMemStringify(Mem * pMem, u8 bForce) +sqlVdbeMemStringify(Mem * pMem) { int fg = pMem->flags; const int nByte = 32; @@ -294,18 +294,20 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce) } if (fg & MEM_Int) { sql_snprintf(nByte, pMem->z, "%lld", pMem->u.i); + pMem->flags &= ~MEM_Int; } else if ((fg & MEM_UInt) != 0) { sql_snprintf(nByte, pMem->z, "%llu", pMem->u.u); + pMem->flags &= ~MEM_UInt; } else if ((fg & MEM_Bool) != 0) { sql_snprintf(nByte, pMem->z, "%s", pMem->u.b ? "true" : "false"); + pMem->flags &= ~MEM_Bool; } else { assert(fg & MEM_Real); sql_snprintf(nByte, pMem->z, "%!.15g", pMem->u.r); + pMem->flags &= ~MEM_Real; } pMem->n = sqlStrlen30(pMem->z); pMem->flags |= MEM_Str | MEM_Term; - if (bForce) - pMem->flags &= ~(MEM_Int | MEM_UInt | MEM_Real); return 0; } @@ -1141,7 +1143,7 @@ valueToText(sql_value * pVal) pVal->flags |= MEM_Str; sqlVdbeMemNulTerminate(pVal); /* IMP: R-31275-44060 */ } else { - sqlVdbeMemStringify(pVal, 0); + sqlVdbeMemStringify(pVal); assert(0 == (1 & SQL_PTR_TO_INT(pVal->z))); } return pVal->z; -- GitLab