[Tarantool-patches] [PATCH v5 35/52] sql: introduce mem_set_frame()

[Mergen Imeev]

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

On Tue, Apr 13, 2021 at 01:37:08AM +0200, Vladislav Shpilevoy wrote:
> Good job on the patch!
> 
> > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> > index c21a6576f..7f1e0bcbe 100644
> > --- a/src/box/sql/vdbe.c
> > +++ b/src/box/sql/vdbe.c
> > @@ -4163,8 +4163,7 @@ case OP_Program: {        /* jump */
> >  			goto no_mem;
> >  		}
> >  		mem_destroy(pRt);
> 
> You can remove this destroy call.
> 
Fixed.

> > -		pRt->flags = MEM_Frame;
> > -		pRt->u.pFrame = pFrame;
> > +		mem_set_frame(pRt, pFrame);
> >  
> >  		pFrame->v = p;
> >  		pFrame->nChildMem = nMem;
> > 


Diff:


diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7a3170614..d5b8033a4 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4206,7 +4206,6 @@ case OP_Program: {        /* jump */
 		if (!pFrame) {
 			goto no_mem;
 		}
-		mem_destroy(pRt);
 		mem_set_frame(pRt, pFrame);
 
 		pFrame->v = p;



New patch:


commit 24a0ee0f12e15b8504c04ea34d973bcfc6a922a1
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Tue Mar 16 14:40:37 2021 +0300

    sql: introduce mem_set_frame()
    
    This patch introduces mem_set_frame() function. This function clears the
    MEM and sets a frame to MEM. Frames used for internal VDBE operations.
    
    Part of #5818

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 6c5c4f391..8e13131f1 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -461,6 +461,14 @@ mem_set_ptr(struct Mem *mem, void *ptr)
 	mem->u.p = ptr;
 }
 
+void
+mem_set_frame(struct Mem *mem, struct VdbeFrame *frame)
+{
+	mem_clear(mem);
+	mem->flags = MEM_Frame;
+	mem->u.pFrame = frame;
+}
+
 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 f4cdbafd2..36b7d3eca 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -542,6 +542,10 @@ mem_set_invalid(struct Mem *mem);
 void
 mem_set_ptr(struct Mem *mem, void *ptr);
 
+/** Clear MEM and set frame to be its value. */
+void
+mem_set_frame(struct Mem *mem, struct VdbeFrame *frame);
+
 /**
  * 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 e3d7456f3..d5b8033a4 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4206,9 +4206,7 @@ case OP_Program: {        /* jump */
 		if (!pFrame) {
 			goto no_mem;
 		}
-		mem_destroy(pRt);
-		pRt->flags = MEM_Frame;
-		pRt->u.pFrame = pFrame;
+		mem_set_frame(pRt, pFrame);
 
 		pFrame->v = p;
 		pFrame->nChildMem = nMem;