[Tarantool-patches] [PATCH v5 34/52] sql: refactor mem_set_ptr()

[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!
>
> I think mem_set_ptr() name was better. Was shorter and still easy to
> understand.
>
>> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
>> index fd2710478..3701741ef 100644
>> --- a/src/box/sql/mem.c
>> +++ b/src/box/sql/mem.c
>> @@ -595,6 +595,15 @@ mem_set_undefined(struct Mem *mem)
>>  	mem->field_type = field_type_MAX;
>>  }
>>  
>> +void
>> +mem_set_pointer(struct Mem *mem, void *ptr)
>> +{
>> +	mem_clear(mem);
>> +	mem->flags = MEM_Ptr;
>> +	mem->u.p = ptr;
>> +	mem->field_type = field_type_MAX;
>
> mem_clear() already sets the type.
Fixed.


New patch:

commit 6d76eee6ea147188aced9dfa18603bd7d2c2e744
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Tue Mar 16 14:36:24 2021 +0300

    sql: refactor mem_set_ptr()
    
    This patch refactors mem_set_ptr() function. Also, it moves the function
    to all others mem_set_*() functions.
    
    Part of #5818

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index b97904f22..4091161ce 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -596,6 +596,14 @@ mem_set_invalid(struct Mem *mem)
 	mem->flags = MEM_Undefined;
 }
 
+void
+mem_set_ptr(struct Mem *mem, void *ptr)
+{
+	mem_clear(mem);
+	mem->flags = MEM_Ptr;
+	mem->u.p = ptr;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
@@ -2241,14 +2249,6 @@ sqlVdbeMemClearAndResize(Mem * pMem, int szNew)
 	return 0;
 }
 
-void
-mem_set_ptr(struct Mem *mem, void *ptr)
-{
-	mem_destroy(mem);
-	mem->flags = MEM_Ptr;
-	mem->u.p = ptr;
-}
-
 /*
  * Free an sql_value object
  */
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index fe3d8d98f..ae5d78ecb 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -356,6 +356,10 @@ mem_set_array_allocated(struct Mem *mem, char *value, uint32_t size);
 void
 mem_set_invalid(struct Mem *mem);
 
+/** Clear MEM and set pointer to be its value. */
+void
+mem_set_ptr(struct Mem *mem, void *ptr);
+
 /**
  * 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
@@ -625,14 +629,6 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type);
 
 int sqlVdbeMemClearAndResize(struct Mem * pMem, int n);
 
-/**
- * Set VDBE memory register with given pointer as a data.
- * @param mem VDBE memory register to update.
- * @param ptr Pointer to use.
- */
-void
-mem_set_ptr(struct Mem *mem, void *ptr);
-
 void sqlValueFree(struct Mem *);
 struct Mem *sqlValueNew(struct sql *);
 
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 0031d2248..c21a6576f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2485,8 +2485,7 @@ case OP_OpenTEphemeral: {
 
 	if (space == NULL)
 		goto abort_due_to_error;
-	aMem[pOp->p1].u.p = space;
-	aMem[pOp->p1].flags = MEM_Ptr;
+	mem_set_ptr(&aMem[pOp->p1], space);
 	break;
 }