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

[Kirill Shcherbatov]

> Why do you need snprintf("%s", str)? It is the same as just 'str'.
> is enough to set an error. If zErrMsg was not terminated by zero, snprintf("%s")
> would not work as well. But zErrMsg is terminated.
I  wasn't sure that this diag_set build print string object. It doesn't matter. I've fixed this. 

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

--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -453,5 +453,6 @@ sql_parser_destroy(Parse *parser)
                db->lookaside.bDisable -= parser->disableLookaside;
        }
        parser->disableLookaside = 0;
+       sqlite3DbFree(db, parser->zErrMsg);
        region_destroy(&parser->region);
 }


--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -815,6 +815,7 @@ transferParseError(Parse * pTo, Parse * pFrom)
        } else {
                sqlite3DbFree(pFrom->db, pFrom->zErrMsg);
        }
+       pFrom->zErrMsg = NULL;
 }
 

--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1856,13 +1856,13 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
 
        sql_resolve_self_reference(&parser, &dummy_table, NC_IsCheck, NULL,
                                   expr_list);
-
-       sql_parser_destroy(&parser);
+       int rc = 0;
        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);
-               return -1;
+               rc = -1;
        }
-       return 0;
+       sql_parser_destroy(&parser);
+       return rc;
 }