[Tarantool-patches] [PATCH v5 33/52] sql: introduce mem_set_invalid()

[imeevma at tarantool.org]

Thank you for the review! My answer and new patch below.


On 30.03.2021 02:06, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> On 23.03.2021 10:36, Mergen Imeev via Tarantool-patches wrote:
>> This patch introduces mem_set_undefined() function. Function
>> mem_set_undefined() invalidates MEM. It does not clears MEM prior to
>> invalidating it.
>
> Why does not it clear it?
Usually MEM_Undefined is just set to MEM without any other changes. I cannot say
for sure that all MEMs that have this flag set did not have MEM_Dyn, MEM_Frame
or MEM_Agg set previously. I added clear() to the function.


New patch:

commit 07b94caea28f23d61ba6f8a31d0ce70c7829dde6
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Tue Mar 16 14:26:55 2021 +0300

    sql: introduce mem_set_invalid()
    
    This patch introduces mem_set_invalid() function. This function clears
    MEM and invalidates it.
    
    Part of #5818

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 61849cde7..b97904f22 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -589,6 +589,13 @@ mem_set_array_allocated(struct Mem *mem, char *value, uint32_t size)
 	set_msgpack_value(mem, value, size, 0, FIELD_TYPE_ARRAY);
 }
 
+void
+mem_set_invalid(struct Mem *mem)
+{
+	mem_clear(mem);
+	mem->flags = MEM_Undefined;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index b3a602f6e..fe3d8d98f 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -352,6 +352,10 @@ mem_set_array_dynamic(struct Mem *mem, char *value, uint32_t size);
 void
 mem_set_array_allocated(struct Mem *mem, char *value, uint32_t size);
 
+/** Clear MEM and set it to invalid state. */
+void
+mem_set_invalid(struct Mem *mem);
+
 /**
  * Copy content of MEM from one MEM to another. In case source MEM contains
  * string or binary and allocation type is not STATIC, this value is copied to
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9434a4d06..0031d2248 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -77,7 +77,7 @@ sqlVdbeMemAboutToChange(Vdbe * pVdbe, Mem * pMem)
 		if (mem_is_bytes(pX) && !mem_is_ephemeral(pX) &&
 		    !mem_is_static(pX)) {
 			if (pX->pScopyFrom == pMem) {
-				pX->flags |= MEM_Undefined;
+				mem_set_invalid(pX);
 				pX->pScopyFrom = 0;
 			}
 		}
@@ -629,7 +629,7 @@ case OP_Return: {           /* in1 */
 	pIn1 = &aMem[pOp->p1];
 	assert(mem_is_uint(pIn1));
 	pOp = &aOp[pIn1->u.u];
-	pIn1->flags = MEM_Undefined;
+	mem_set_invalid(pIn1);
 	break;
 }
 
@@ -672,7 +672,7 @@ case OP_EndCoroutine: {           /* in1 */
 	assert(pCaller->opcode==OP_Yield);
 	assert(pCaller->p2>=0 && pCaller->p2<p->nOp);
 	pOp = &aOp[pCaller->p2 - 1];
-	pIn1->flags = MEM_Undefined;
+	mem_set_invalid(pIn1);
 	break;
 }
 
@@ -4181,8 +4181,8 @@ case OP_Program: {        /* jump */
 
 		pEnd = &VdbeFrameMem(pFrame)[pFrame->nChildMem];
 		for(pMem=VdbeFrameMem(pFrame); pMem!=pEnd; pMem++) {
-			pMem->flags = MEM_Undefined;
-			pMem->db = db;
+			mem_create(pMem);
+			mem_set_invalid(pMem);
 		}
 	} else {
 		pFrame = pRt->u.pFrame;
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index c294a0286..a5e78be06 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1592,7 +1592,7 @@ sqlVdbeMakeReady(Vdbe * p,	/* The VDBE */
 		p->nMem = nMem;
 		for (int i = 0; i < nMem; ++i) {
 			mem_create(&p->aMem[i]);
-			p->aMem[i].flags = MEM_Undefined;
+			mem_set_invalid(&p->aMem[i]);
 		}
 		memset(p->apCsr, 0, nCursor * sizeof(VdbeCursor *));
 	}