[tarantool-patches] [PATCH v2 08/15] sql: fixes errors

[Stanislav Zudin <szudin@tarantool.org>]

uint64 to double casting
string to int64 conversion
sqlSubInt64
sql_value_type()
wrong arguments order in arithmetic operations.

Part of #3810
---
 src/box/sql/util.c    | 59 +++++++++++++++++++++++--------------------
 src/box/sql/vdbe.c    | 10 ++++----
 src/box/sql/vdbeInt.h |  2 ++
 src/box/sql/vdbeapi.c |  3 ++-
 src/box/sql/vdbemem.c |  2 +-
 5 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index 3786c5083..d585dc0d5 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -599,6 +599,7 @@ sqlAtoF(const char *z, double *pResult, int length)
 enum atoi_result
 sql_atoi64(const char *z, int64_t *val, int length)
 {
+	const char* expected_end = z + length;
 	int neg = 0;		/* assume positive */
 	const char *zEnd = z + length;
 	int incr = 1;
@@ -614,7 +615,7 @@ sql_atoi64(const char *z, int64_t *val, int length)
 
 	char* end = NULL;
 	u64 u = strtoull(z, &end, 10);
-	if (end == z)
+	if (end < expected_end)
 		return ATOI_OVERFLOW;
 	if (errno == ERANGE)
 		return ATOI_OVERFLOW;
@@ -1260,6 +1261,25 @@ static u64 mod64(i64 v, bool is_signed)
 		return (u64)v;
 }
 
+static enum arithmetic_result
+apply_sign(i64* pOut, u64 value, bool is_neg)
+{
+	if (is_neg) {
+		if (value > INT64_MIN_MOD)
+			return ATHR_OVERFLOW;
+		else if (value == INT64_MIN_MOD)
+			*pOut = (i64)value;
+		else
+			*pOut = -(i64)value;
+
+		return ATHR_SIGNED;
+	}
+
+	*pOut = (i64) value;
+	return (value > INT64_MAX) ? ATHR_UNSIGNED
+				   : ATHR_SIGNED;
+}
+
 /*
  * Attempt to add, substract, or multiply the 64-bit value iB against
  * the other 64-bit integer at *pA and store the result in *pA.
@@ -1298,7 +1318,7 @@ sqlAddInt64(i64 * pA, bool is_signedA, i64 iB, bool is_signedB)
 			if (sum == INT64_MIN_MOD) {
 				*pA = INT64_MIN;
 			} else {
-				assert(sum < INT64_MAX);
+				assert(sum <= INT64_MAX);
 				*pA = -(i64)sum;
 			}
 			return ATHR_SIGNED;
@@ -1351,32 +1371,17 @@ sqlSubInt64(i64 * pA, bool is_signedA, i64 iB, bool is_signedB)
 		return sqlAddInt64(pA, is_signedA, uB, is_signedB);
 	} else {
 		/* Both iA & iB are positive */
-		if ((u64)iA < (u64)iB)
-			return  ATHR_OVERFLOW;
-		u64 val = (u64)iA - (u64)iB;
-		*pA = (i64)val;
-		return (val > INT64_MAX) ? ATHR_UNSIGNED
-					 : ATHR_SIGNED;
-	}
-}
-
-static enum arithmetic_result
-apply_sign(i64* pOut, u64 value, bool is_neg)
-{
-	if (is_neg) {
-		if (value > INT64_MIN_MOD)
-			return ATHR_OVERFLOW;
-		else if (value == INT64_MIN_MOD)
-			*pOut = (i64)value;
-		else
-			*pOut = -(i64)value;
-
-		return ATHR_SIGNED;
+		if ((u64)iA < (u64)iB) {
+			/* subtract with sign changing */
+			u64 val = (u64)iB - (u64)iA;
+			return apply_sign(pA, val, true);
+		} else {
+			u64 val = (u64)iA - (u64)iB;
+			*pA = (i64)val;
+			return (val > INT64_MAX) ? ATHR_UNSIGNED
+						 : ATHR_SIGNED;
+		}
 	}
-
-	*pOut = (i64) value;
-	return (value > INT64_MAX) ? ATHR_UNSIGNED
-			  	   : ATHR_SIGNED;
 }
 
 enum arithmetic_result
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index d4bd845fb..ad2ce1787 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1677,11 +1677,11 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		bIntint = 1;
 		enum arithmetic_result arr;
 		switch( pOp->opcode) {
-		case OP_Add:       arr = sqlAddInt64(&iB, is_signedA, iA, is_signedB); break;
-		case OP_Subtract:  arr = sqlSubInt64(&iB, is_signedA, iA, is_signedB); break;
-		case OP_Multiply:  arr = sqlMulInt64(&iB, is_signedA, iA, is_signedB); break;
-		case OP_Divide:    arr = sqlDivInt64(&iB, is_signedA, iA, is_signedB); break;
-		default: 	   arr = sqlRemInt64(&iB, is_signedA, iA, is_signedB); break;
+		case OP_Add:       arr = sqlAddInt64(&iB, is_signedB, iA, is_signedA); break;
+		case OP_Subtract:  arr = sqlSubInt64(&iB, is_signedB, iA, is_signedA); break;
+		case OP_Multiply:  arr = sqlMulInt64(&iB, is_signedB, iA, is_signedA); break;
+		case OP_Divide:    arr = sqlDivInt64(&iB, is_signedB, iA, is_signedA); break;
+		default: 	   arr = sqlRemInt64(&iB, is_signedB, iA, is_signedA); break;
 		}
 
 		switch(arr){
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 42f22df52..2076a9a14 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -251,6 +251,8 @@ struct Mem {
 #define MEM_Unsigned  0x20000	/* Value is unsigned integer.
 				 * Combine this flag with MEM_Int
 				 * if necessary */
+#define MEM_UInt (MEM_Int | MEM_Unsigned)
+
 #ifdef SQL_OMIT_INCRBLOB
 #undef MEM_Zero
 #define MEM_Zero 0x0000
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 6a3413954..06140569c 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -319,7 +319,8 @@ sql_value_type(sql_value * pVal)
 	 * type bits, to make them applicable for
 	 * array indexing.
 	 */
-	u32 offset = (pVal->flags >> 12) | (pVal->flags & MEM_PURE_TYPE_MASK);
+	u32 offset = ((pVal->flags & MEM_Unsigned) >> 12) |
+		     (pVal->flags & MEM_PURE_TYPE_MASK);
 	assert(offset < 0x40);
 	return aType[offset];
 }
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 2805d7a01..9e6d52b47 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -489,7 +489,7 @@ sqlVdbeRealValue(Mem * pMem, double *v)
 	if (pMem->flags & MEM_Real) {
 		*v = pMem->u.r;
 		return 0;
-	} else if (pMem->flags & (MEM_Int | MEM_Unsigned)) {
+	} else if ((pMem->flags & MEM_UInt) == MEM_UInt) {
 		*v = (double)(u64)pMem->u.i;
 		return 0;
 	} else if (pMem->flags & MEM_Int) {
-- 
2.17.1