[tarantool-patches] Re: [PATCH v2 03/11] sql: fix leak on CREATE TABLE and resolve self ref

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch! See 1 comment below.

On 09/06/2018 12:32, Kirill Shcherbatov wrote:
> ---
>   src/box/sql.c       |  8 ++++++--
>   src/box/sql/build.c | 14 +++++---------
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 93fca68..72fd5cc 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -1860,8 +1860,12 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
>   	sql_parser_destroy(&parser);
>   	if (parser.rc != SQLITE_OK) {
>   		/* Tarantool error may be already set with diag. */
> -		if (parser.rc != SQL_TARANTOOL_ERROR)
> -			diag_set(ClientError, ER_SQL, parser.zErrMsg);
> +		if (parser.rc != SQL_TARANTOOL_ERROR) {
> +			char *error = tt_static_buf();
> +			snprintf(error, TT_STATIC_BUF_LEN, "%s", parser.zErrMsg);
> +			diag_set(ClientError, ER_SQL, error);

Why do you need snprintf("%s", str)? It is the same as just 'str'.

	diag_set(ClientError, ER_SQL, parser.zErrMsg);

is enough to set an error. If zErrMsg was not terminated by zero, snprintf("%s")
would not work as well. But zErrMsg is terminated.

And why do you destroy zErrMsg here? Why not in sql_parser_destroy()?
zErrMsg is struct Parse member.

> +			sqlite3DbFree(db, parser.zErrMsg);
> +		}
>   		return -1;
>   	}
>   	return 0;