Re: [Tarantool-patches] [PATCH v1 2/3] sql: refactor sql_vdbe_mem_encode_tuple()

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 356b2c7be..2ba4135b0 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -3013,31 +3013,31 @@ mem_to_mpstream(const struct Mem *var, struct mpstream *stream)
>  }
>  
>  char *
> -sql_vdbe_mem_encode_tuple(struct Mem *fields, uint32_t field_count,
> -			  uint32_t *tuple_size, struct region *region)
> +mem_encode_array(const struct Mem *mems, uint32_t count, uint32_t *size,
> +		 struct region *region)
>  {
>  	size_t used = region_used(region);
>  	bool is_error = false;
>  	struct mpstream stream;
>  	mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
>  		      set_encode_error, &is_error);
> -	mpstream_encode_array(&stream, field_count);
> -	for (struct Mem *field = fields; field < fields + field_count; field++)
> -		mem_to_mpstream(field, &stream);
> +	mpstream_encode_array(&stream, count);
> +	for (const struct Mem *mem = mems; mem < mems + count; mem++)
> +		mem_to_mpstream(mem, &stream);
>  	mpstream_flush(&stream);
>  	if (is_error) {

If some memory was allocated/reserved before mpstream_flush() failed (inside
of mem_to_mpstream()), then the region might leak. I would recommend to add a
truncate here.

>  		diag_set(OutOfMemory, stream.pos - stream.buf,
>  			 "mpstream_flush", "stream");
>  		return NULL;
>  	}