Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix a segfault in hex() on receiving zeroblob

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

Thanks for the patch!

> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index b137c6125..d182bb313 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -1221,14 +1221,22 @@ hexFunc(sql_context * context, int argc, sql_value ** argv)
>  	UNUSED_PARAMETER(argc);
>  	pBlob = mem_as_bin(argv[0]);
>  	n = mem_len_unsafe(argv[0]);
> +	assert((argv[0]->flags & MEM_Zero) == 0 ||
> +	       argv[0]->type == MEM_TYPE_BIN);
> +	int zero_len = (argv[0]->flags & MEM_Zero) == 0 ? 0 : argv[0]->u.nZero;
>  	assert(pBlob == mem_as_bin(argv[0]));	/* No encoding change */
>  	z = zHex = contextMalloc(context, ((i64) n) * 2 + 1);
>  	if (zHex) {
> -		for (i = 0; i < n; i++, pBlob++) {
> +		for (i = 0; i < n - zero_len; i++, pBlob++) {
>  			unsigned char c = *pBlob;
>  			*(z++) = hexdigits[(c >> 4) & 0xf];
>  			*(z++) = hexdigits[c & 0xf];
>  		}
> +		for (; i < n; ++i) {
> +			assert((argv[0]->flags & MEM_Zero) != 0);

1. This assert can be out of the loop. It does not depend on z or i.

2. The loop could be replaced with memset().

> +			*(z++) = '0';
> +			*(z++) = '0';
> +		}