[Tarantool-patches] [PATCH v5 28/52] sql: introduce mem_copy_str() and mem_copy_str0()

[Vladislav Shpilevoy]

Good job on the patch!

> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 5cf067453..ded20315b 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -252,6 +252,34 @@ mem_set_str0_allocated(struct Mem *mem, char *value)
>  	mem->flags |= MEM_Term;
>  }
>  
> +int
> +mem_copy_str(struct Mem *mem, const char *value, uint32_t len)
> +{
> +	if ((mem->flags & (MEM_Agg | MEM_Frame)) != 0)
> +		mem_clear(mem);
> +	bool is_own_value = (mem->flags & (MEM_Str | MEM_Blob)) != 0 &&
> +			    mem->z == value;
> +	if (sqlVdbeMemGrow(mem, len, is_own_value) != 0)
> +		return -1;

I would propose this change:

====================
@@ -256,14 +256,14 @@ mem_set_str0_allocated(struct Mem *mem, char *value)
 int
 mem_copy_str(struct Mem *mem, const char *value, uint32_t len)
 {
-	if ((mem->flags & (MEM_Agg | MEM_Frame)) != 0)
-		mem_clear(mem);
-	bool is_own_value = (mem->flags & (MEM_Str | MEM_Blob)) != 0 &&
-			    mem->z == value;
-	if (sqlVdbeMemGrow(mem, len, is_own_value) != 0)
+	if ((mem->flags & (MEM_Str | MEM_Blob)) != 0 && mem->z == value) {
+		/* Own value, but might be ephemeral. Make it own if so. */
+		return sqlVdbeMemGrow(mem, len, 1);
+	}
+	mem_clear(mem);
+	if (sqlVdbeMemGrow(mem, len, 0) != 0)
 		return -1;
-	if (!is_own_value)
-		memcpy(mem->z, value, len);
+	memcpy(mem->z, value, len);
 	mem->n = len;
 	mem->flags = MEM_Str;
 	mem->field_type = FIELD_TYPE_STRING;
====================

The same for the bins.

> +	if (!is_own_value)
> +		memcpy(mem->z, value, len);
> +	mem->n = len;
> +	mem->flags = MEM_Str;
> +	mem->field_type = FIELD_TYPE_STRING;
> +	return 0;
> +}