From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id A736B2265B for ; Wed, 24 Jul 2019 07:42:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 26KfEu4aKDzr for ; Wed, 24 Jul 2019 07:42:51 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 0B11521CC2 for ; Wed, 24 Jul 2019 07:42:50 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 1/5] sql: always erase numeric flag after stringifying Date: Wed, 24 Jul 2019 14:42:43 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik 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 | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c8887f9b7..9f3879910 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; } /* @@ -333,7 +333,7 @@ mem_apply_type(struct Mem *record, enum field_type type) */ if ((record->flags & MEM_Str) == 0) { if ((record->flags & (MEM_Real | MEM_Int))) - sqlVdbeMemStringify(record, 1); + sqlVdbeMemStringify(record); } record->flags &= ~(MEM_Real | MEM_Int); return 0; @@ -2144,7 +2144,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|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); @@ -2152,7 +2152,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if ((flags3 & MEM_Str)==0 && (flags3 & (MEM_Int|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 6bfeecc85..7d2470b07 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -469,7 +469,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 *); int sqlVdbeMemIntegerify(Mem *, bool is_forced); int sqlVdbeRealValue(Mem *, double *); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 4e4bd597d..d85148bc3 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -275,7 +275,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; @@ -292,16 +292,17 @@ 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_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_Real); return 0; } @@ -1107,7 +1108,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; -- 2.15.1