From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3FB8B2A847 for ; Fri, 24 Aug 2018 12:33:38 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JYPwMke8x1TQ for ; Fri, 24 Aug 2018 12:33:38 -0400 (EDT) Received: from smtp20.mail.ru (smtp20.mail.ru [94.100.179.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id C38F52A8CA for ; Fri, 24 Aug 2018 12:33:37 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: remove struct Enc References: <792834dabd4287aeb213202581de02b23a4c4557.1534508548.git.kshcherbatov@tarantool.org> <59B7B0ED-9BE2-402D-9953-0347E576BE00@tarantool.org> <6bdaa379-f063-0364-49ca-a5ad573c6975@tarantool.org> From: Vladislav Shpilevoy Message-ID: <876e556e-b1ad-e4e4-8efb-1e7c9046ac12@tarantool.org> Date: Fri, 24 Aug 2018 19:33:35 +0300 MIME-Version: 1.0 In-Reply-To: <6bdaa379-f063-0364-49ca-a5ad573c6975@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Kirill Shcherbatov , "n.pettik" Thanks for the patch! > diff --git a/src/box/sql.c b/src/box/sql.c > index ae12cae..28ad9d3 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -1315,10 +1261,10 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf) > base_len += 1; > if (default_str != NULL) > base_len += 1; > - p = enc->encode_map(p, base_len); > - p = enc->encode_str(p, "name", 4); > - p = enc->encode_str(p, field->name, strlen(field->name)); > - p = enc->encode_str(p, "type", 4); > + mpstream_encode_map(&stream, base_len); > + mpstream_encode_str(&stream, "name", strlen("name")); > + mpstream_encode_str(&stream, field->name, strlen(field->name)); > + mpstream_encode_str(&stream, "type", strlen("type")); 1. This construction 'encode_str(stream, str, strlen(str))' is extremely frequent in the overall project. Please, in the previous commit split mpstream_encode_str into 2 functions: mpstream_encode_str(stream, str) mpstream_encode_strn(stream, str, len) The first one calls mpstream_encode_strn(stream, str, strlen(str)) and is static inline in the header. With those SQL encoders we can use mpstream_encode_str(stream, str). > if (i == pk_forced_int) { > t = "integer"; > } else { > @@ -1329,108 +1275,149 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf) > } > > assert(def->fields[i].is_nullable == > - action_is_nullable(def->fields[i].nullable_action)); > - p = enc->encode_str(p, t, strlen(t)); > - p = enc->encode_str(p, "affinity", 8); > - p = enc->encode_uint(p, def->fields[i].affinity); > - p = enc->encode_str(p, "is_nullable", 11); > - p = enc->encode_bool(p, def->fields[i].is_nullable); > - p = enc->encode_str(p, "nullable_action", 15); > + action_is_nullable(def->fields[i]. > + nullable_action)); > + mpstream_encode_str(&stream, t, strlen(t)); > + mpstream_encode_str(&stream, "affinity", strlen("affinity")); > + mpstream_encode_uint(&stream, > + def->fields[i].affinity); > + mpstream_encode_str(&stream, "is_nullable", > + strlen("is_nullable")); > + mpstream_encode_bool(&stream, > + def->fields[i].is_nullable); > + mpstream_encode_str(&stream, "nullable_action", > + strlen("nullable_action")); > assert(def->fields[i].nullable_action < on_conflict_action_MAX); > const char *action = > on_conflict_action_strs[def->fields[i].nullable_action]; > - p = enc->encode_str(p, action, strlen(action)); > + mpstream_encode_str(&stream, action, strlen(action)); > if (cid != COLL_NONE) { > - p = enc->encode_str(p, "collation", strlen("collation")); > - p = enc->encode_uint(p, cid); > + mpstream_encode_str(&stream, "collation", > + strlen("collation")); > + mpstream_encode_uint(&stream, cid); > } > if (default_str != NULL) { > - p = enc->encode_str(p, "default", strlen("default")); > - p = enc->encode_str(p, default_str, strlen(default_str)); > + mpstream_encode_str(&stream, "default", > + strlen("default")); > + mpstream_encode_str(&stream, default_str, > + strlen(default_str)); > } > } > - return (int)(p - base); > + mpstream_flush(&stream); > + if (is_error) > + return NULL; 2. Neither region_alloc_cb nor reserve_cb set diagnostics, so here you return NULL, but have an empty diag. > + size_t sz = region_used(region) - used; > + char *raw = region_join(region, sz); > + if (raw == NULL) > + diag_set(OutOfMemory, sz, "region_join", "raw"); > + else > + *size = sz; > + return raw; > } > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index dddeb12..462b6e6 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -1603,33 +1614,53 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) > fkey_action_strs[fk->on_delete], P4_STATIC); > sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 6, 0, > fkey_action_strs[fk->on_update], P4_STATIC); > - size_t parent_size = fkey_encode_links(fk, FIELD_LINK_PARENT, NULL); > - size_t child_size = fkey_encode_links(fk, FIELD_LINK_CHILD, NULL); > + struct region *region = &parse_context->region; > + uint32_t parent_links_size = 0; > + char *parent_links = > + fkey_encode_links(region, fk, FIELD_LINK_PARENT, > + &parent_links_size); > + if (parent_links == NULL) > + goto error; > + uint32_t child_links_size = 0; > + char *child_links = > + fkey_encode_links(region, fk, FIELD_LINK_CHILD, > + &child_links_size); > + if (child_links == NULL) > + goto error; > /* > * We are allocating memory for both parent and child > * arrays in the same chunk. Thus, first OP_Blob opcode > * interprets it as static memory, and the second one - > * as dynamic and releases memory. > */ > - char *parent_links = sqlite3DbMallocRaw(parse_context->db, > - parent_size + child_size); > - if (parent_links == NULL) { > - sqlite3DbFree(parse_context->db, (void *) name_copy); > + char *raw = sqlite3DbMallocRaw(parse_context->db, > + parent_links_size + child_links_size); > + if (raw == NULL) { > + sqlite3DbFree(parse_context->db, name_copy); 3. You shall not free name copy here. It is added to Vdbe in the first lines of the function as P4_DYNAMIC and will be freed on Vdbe destruction. > return; > } 4. I have pushed my fixes on the branch. Please, look, squash, and fix other things. My diff is below: =============================================================== commit 19fb0e03bf99a74e41349af4553299899f32deeb Author: Vladislav Shpilevoy Date: Fri Aug 24 16:59:22 2018 +0300 Review fixes diff --git a/src/box/sql.c b/src/box/sql.c index 28ad9d37f..790c0529a 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -1220,11 +1220,11 @@ static const char *convertSqliteAffinity(int affinity, bool allow_nulls) } } +/** Callback to forward and error from mpstream methods. */ static void set_encode_error(void *error_ctx) { - bool *is_error = error_ctx; - *is_error = true; + *(bool *)error_ctx = true; } char * @@ -1273,18 +1273,14 @@ sql_encode_table(struct region *region, struct Table *table, uint32_t *size) convertSqliteAffinity(affinity, def->fields[i].is_nullable); } - assert(def->fields[i].is_nullable == - action_is_nullable(def->fields[i]. - nullable_action)); + action_is_nullable(def->fields[i].nullable_action)); mpstream_encode_str(&stream, t, strlen(t)); mpstream_encode_str(&stream, "affinity", strlen("affinity")); - mpstream_encode_uint(&stream, - def->fields[i].affinity); + mpstream_encode_uint(&stream, def->fields[i].affinity); mpstream_encode_str(&stream, "is_nullable", strlen("is_nullable")); - mpstream_encode_bool(&stream, - def->fields[i].is_nullable); + mpstream_encode_bool(&stream, def->fields[i].is_nullable); mpstream_encode_str(&stream, "nullable_action", strlen("nullable_action")); assert(def->fields[i].nullable_action < on_conflict_action_MAX); @@ -1306,12 +1302,10 @@ sql_encode_table(struct region *region, struct Table *table, uint32_t *size) mpstream_flush(&stream); if (is_error) return NULL; - size_t sz = region_used(region) - used; - char *raw = region_join(region, sz); + *size = region_used(region) - used; + char *raw = region_join(region, *size); if (raw == NULL) - diag_set(OutOfMemory, sz, "region_join", "raw"); - else - *size = sz; + diag_set(OutOfMemory, *size, "region_join", "raw"); return raw; } @@ -1326,12 +1320,16 @@ sql_encode_table_opts(struct region *region, struct Table *table, mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb, set_encode_error, &is_error); bool is_view = false; - bool has_checks = false; + int checks_cnt = 0; + struct ExprList_item *a; if (table != NULL) { is_view = table->def->opts.is_view; - has_checks = table->def->opts.checks != NULL; + struct ExprList *checks = table->def->opts.checks; + if (checks != NULL) { + checks_cnt = checks->nExpr; + a = checks->a; + } } - uint32_t checks_cnt = has_checks ? table->def->opts.checks->nExpr : 0; mpstream_encode_map(&stream, 1 + is_view + (checks_cnt > 0)); mpstream_encode_str(&stream, "sql", strlen("sql")); @@ -1340,39 +1338,32 @@ sql_encode_table_opts(struct region *region, struct Table *table, mpstream_encode_str(&stream, "view", strlen("view")); mpstream_encode_bool(&stream, true); } - if (checks_cnt == 0) - goto finish; - - /* Encode checks. */ - struct ExprList_item *a = table->def->opts.checks->a; - mpstream_encode_str(&stream, "checks", strlen("checks")); - mpstream_encode_array(&stream, checks_cnt); - for (uint32_t i = 0; i < checks_cnt && !is_error; ++i) { - int items = (a[i].pExpr != NULL) + (a[i].zName != NULL); + if (checks_cnt > 0) { + mpstream_encode_str(&stream, "checks", strlen("checks")); + mpstream_encode_array(&stream, checks_cnt); + } + for (int i = 0; i < checks_cnt && !is_error; ++i, ++a) { + int items = (a->pExpr != NULL) + (a->zName != NULL); mpstream_encode_map(&stream, items); - assert(a[i].pExpr != NULL); - struct Expr *pExpr = a[i].pExpr; + assert(a->pExpr != NULL); + struct Expr *pExpr = a->pExpr; assert(pExpr->u.zToken != NULL); mpstream_encode_str(&stream, "expr", strlen("expr")); mpstream_encode_str(&stream, pExpr->u.zToken, strlen(pExpr->u.zToken)); - if (a[i].zName != NULL) { + if (a->zName != NULL) { mpstream_encode_str(&stream, "name", strlen("name")); - mpstream_encode_str(&stream, a[i].zName, - strlen(a[i].zName)); + mpstream_encode_str(&stream, a->zName, + strlen(a->zName)); } } - -finish: mpstream_flush(&stream); if (is_error) return NULL; - size_t sz = region_used(region) - used; - char *raw = region_join(region, sz); + *size = region_used(region) - used; + char *raw = region_join(region, *size); if (raw == NULL) - diag_set(OutOfMemory, sz, "region_join", "raw"); - else - *size = sz; + diag_set(OutOfMemory, *size, "region_join", "raw"); return raw; } @@ -1392,18 +1383,16 @@ fkey_encode_links(struct region *region, const struct fkey_def *def, int type, mpstream_flush(&stream); if (is_error) return NULL; - size_t sz = region_used(region) - used; - char *raw = region_join(region, sz); + *size = region_used(region) - used; + char *raw = region_join(region, *size); if (raw == NULL) - diag_set(OutOfMemory, sz, "region_join", "raw"); - else - *size = sz; + diag_set(OutOfMemory, *size, "region_join", "raw"); return raw; } char * sql_encode_index_parts(struct region *region, struct SqliteIndex *index, - uint32_t *size) + uint32_t *size) { size_t used = region_used(region); struct mpstream stream; @@ -1415,13 +1404,12 @@ sql_encode_index_parts(struct region *region, struct SqliteIndex *index, * treat it as strict type, not affinity. */ uint32_t pk_forced_int = UINT32_MAX; - struct SqliteIndex *primary_index = - sqlite3PrimaryKeyIndex(index->pTable); + struct SqliteIndex *pk = sqlite3PrimaryKeyIndex(index->pTable); struct field_def *fields = index->pTable->def->fields; - if (primary_index->def->key_def->part_count == 1) { - int pk = primary_index->def->key_def->parts[0].fieldno; - if (fields[pk].type == FIELD_TYPE_INTEGER) - pk_forced_int = pk; + if (pk->def->key_def->part_count == 1) { + int fieldno = pk->def->key_def->parts[0].fieldno; + if (fields[fieldno].type == FIELD_TYPE_INTEGER) + pk_forced_int = fieldno; } /* gh-2187 @@ -1431,10 +1419,9 @@ sql_encode_index_parts(struct region *region, struct SqliteIndex *index, * data layout. */ struct key_def *key_def = index->def->key_def; - uint32_t part_count = key_def->part_count; struct key_part *part = key_def->parts; - mpstream_encode_array(&stream, part_count); - for (uint32_t i = 0; i < part_count; ++i, ++part) { + mpstream_encode_array(&stream, key_def->part_count); + for (uint32_t i = 0; i < key_def->part_count; ++i, ++part) { uint32_t col = part->fieldno; assert(fields[col].is_nullable == action_is_nullable(fields[col].nullable_action)); @@ -1459,8 +1446,7 @@ sql_encode_index_parts(struct region *region, struct SqliteIndex *index, } mpstream_encode_str(&stream, "is_nullable", strlen("is_nullable")); - mpstream_encode_bool(&stream, - fields[col].is_nullable); + mpstream_encode_bool(&stream, fields[col].is_nullable); mpstream_encode_str(&stream, "nullable_action", strlen("nullable_action")); const char *action_str = @@ -1478,12 +1464,10 @@ sql_encode_index_parts(struct region *region, struct SqliteIndex *index, mpstream_flush(&stream); if (is_error) return NULL; - size_t sz = region_used(region) - used; - char *raw = region_join(region, sz); + *size = region_used(region) - used; + char *raw = region_join(region, *size); if (raw == NULL) - diag_set(OutOfMemory, sz, "region_join", "raw"); - else - *size = sz; + diag_set(OutOfMemory, *size, "region_join", "raw"); return raw; } @@ -1514,12 +1498,10 @@ sql_encode_index_opts(struct region *region, struct SqliteIndex *index, mpstream_flush(&stream); if (is_error) return NULL; - size_t sz = region_used(region) - used; - char *raw = region_join(region, sz); + *size = region_used(region) - used; + char *raw = region_join(region, *size); if (raw == NULL) - diag_set(OutOfMemory, sz, "region_join", "raw"); - else - *size = sz; + diag_set(OutOfMemory, *size, "region_join", "raw"); return raw; } diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 462b6e67b..ab1438db4 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1371,9 +1371,8 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt) int iRecord = (pParse->nMem += 7); struct region *region = &pParse->region; uint32_t table_opts_stmt_sz = 0; - char *table_opts_stmt = - sql_encode_table_opts(region, table, zStmt, - &table_opts_stmt_sz); + char *table_opts_stmt = sql_encode_table_opts(region, table, zStmt, + &table_opts_stmt_sz); if (table_opts_stmt == NULL) goto error; uint32_t table_stmt_sz = 0; @@ -1602,10 +1601,8 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) BOX_FK_CONSTRAINT_ID, 0, constr_tuple_reg, 2, ER_CONSTRAINT_EXISTS, error_msg, - false, OP_NoConflict) != 0) { - sqlite3DbFree(parse_context->db, name_copy); + false, OP_NoConflict) != 0) return; - } sqlite3VdbeAddOp2(vdbe, OP_Bool, 0, constr_tuple_reg + 3); sqlite3VdbeChangeP4(vdbe, -1, (char*)&fk->is_deferred, P4_BOOL); sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 4, 0, @@ -1616,15 +1613,13 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) fkey_action_strs[fk->on_update], P4_STATIC); struct region *region = &parse_context->region; uint32_t parent_links_size = 0; - char *parent_links = - fkey_encode_links(region, fk, FIELD_LINK_PARENT, - &parent_links_size); + char *parent_links = fkey_encode_links(region, fk, FIELD_LINK_PARENT, + &parent_links_size); if (parent_links == NULL) goto error; uint32_t child_links_size = 0; - char *child_links = - fkey_encode_links(region, fk, FIELD_LINK_CHILD, - &child_links_size); + char *child_links = fkey_encode_links(region, fk, FIELD_LINK_CHILD, + &child_links_size); if (child_links == NULL) goto error; /* @@ -1635,10 +1630,8 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) */ char *raw = sqlite3DbMallocRaw(parse_context->db, parent_links_size + child_links_size); - if (raw == NULL) { - sqlite3DbFree(parse_context->db, name_copy); + if (raw == NULL) return; - } memcpy(raw, parent_links, parent_links_size); parent_links = raw; raw += parent_links_size; @@ -1660,7 +1653,6 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) error: parse_context->nErr++; parse_context->rc = SQL_TARANTOOL_ERROR; - sqlite3DbFree(parse_context->db, name_copy); } /** diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index 6a42ed3c6..78790a2bb 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -139,25 +139,25 @@ tarantoolSqlite3IncrementMaxid(uint64_t *space_max_id); /** * Encode format as entry to be inserted to _space on @region. + * @param region Region to use. + * @param table Table to encode. + * @param[out] size Size of result allocation. * - * @param region region to use. - * @param table table to encode. - * @param[out] size size of result allocation. - * @retval NULL on error. - * @retval not NULL pointer to msgpack on success. + * @retval NULL Error. + * @retval not NULL Pointer to msgpack on success. */ char * sql_encode_table(struct region *region, struct Table *table, uint32_t *size); /** * Encode "opts" dictionary for _space entry on @region. + * @param region Region to use. + * @param table Table containing opts to encode. + * @param sql Source request to encode. + * @param[out] size Size of result allocation. * - * @param region region to use. - * @param table table containing opts to encode. - * @param sql source request to encode. - * @param[out] size size of result allocation. - * @retval NULL on error. - * @retval not NULL pointer to msgpack on success + * @retval NULL Error. + * @retval not NULL Pointer to msgpack on success. */ char * sql_encode_table_opts(struct region *region, struct Table *table, @@ -166,13 +166,13 @@ sql_encode_table_opts(struct region *region, struct Table *table, /** * Encode links of given foreign key constraint into MsgPack on * @region. - * - * @param region region to use. + * @param region Wegion to use. * @param def FK def to encode links of. * @param type Links type to encode. - * @param[out] size size of result allocation. - * @retval NULL on error. - * @retval not NULL pointer to msgpack on success + * @param[out] Size size of result allocation. + * + * @retval NULL Error. + * @retval not NULL Pointer to msgpack on success. */ char * fkey_encode_links(struct region *region, const struct fkey_def *def, int type, @@ -181,12 +181,12 @@ fkey_encode_links(struct region *region, const struct fkey_def *def, int type, /** * Encode index parts of given foreign key constraint into * MsgPack on @region. + * @param region Region to use. + * @param index Index to encode. + * @param[out] size Size of result allocation. * - * @param region region to use. - * @param index index to encode. - * @param[out] size size of result allocation. - * @retval NULL on error. - * @retval not NULL pointer to msgpack on success + * @retval NULL Error. + * @retval not NULL Pointer to msgpack on success */ char * sql_encode_index_parts(struct region *region, struct Index *index, diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 6a0708107..f400c2551 100644 --- a/src/box/sql/trigger.c +++ b/src/box/sql/trigger.c @@ -173,16 +173,12 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, { /* Trigger being finished. */ struct sql_trigger *trigger = parse->parsed_ast.trigger; - /* SQL text. */ - char *sql_str = NULL; - /* MsgPack containing SQL options. */ - char *opts_buff = NULL; /* The database. */ struct sqlite3 *db = parse->db; parse->parsed_ast.trigger = NULL; if (NEVER(parse->nErr) || trigger == NULL) - goto triggerfinish_cleanup; + goto cleanup; char *trigger_name = trigger->zName; trigger->step_list = step_list; while (step_list != NULL) { @@ -202,17 +198,18 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, /* Make an entry in the _trigger space. */ struct Vdbe *v = sqlite3GetVdbe(parse); if (v == 0) - goto triggerfinish_cleanup; + goto cleanup; struct Table *sys_trigger = - sqlite3HashFind(&parse->db->pSchema->tblHash, + sqlite3HashFind(&db->pSchema->tblHash, TARANTOOL_SYS_TRIGGER_NAME); if (NEVER(sys_trigger == NULL)) - goto triggerfinish_cleanup; + goto cleanup; - sql_str = sqlite3MPrintf(db, "CREATE TRIGGER %s", token->z); - if (db->mallocFailed) - goto triggerfinish_cleanup; + char *sql_str = + sqlite3MPrintf(db, "CREATE TRIGGER %s", token->z); + if (sql_str == NULL) + goto cleanup; int cursor = parse->nTab++; sqlite3OpenTable(parse, cursor, sys_trigger, OP_OpenWrite); @@ -228,23 +225,24 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, uint32_t opts_buff_sz = 0; char *data = sql_encode_table_opts(&fiber()->gc, NULL, sql_str, &opts_buff_sz); - if (data != NULL) { - opts_buff = sqlite3DbMallocRaw(parse->db, opts_buff_sz); - if (opts_buff != NULL) - memcpy(opts_buff, data, opts_buff_sz); - } else { + sqlite3DbFree(db, sql_str); + if (data == NULL) { parse->nErr++; parse->rc = SQL_TARANTOOL_ERROR; + goto cleanup; } + char *opts_buff = sqlite3DbMallocRaw(db, opts_buff_sz); if (opts_buff == NULL) - goto triggerfinish_cleanup; + goto cleanup; + memcpy(opts_buff, data, opts_buff_sz); - trigger_name = sqlite3DbStrDup(parse->db, trigger_name); - if (trigger_name == NULL) - goto triggerfinish_cleanup; + trigger_name = sqlite3DbStrDup(db, trigger_name); + if (trigger_name == NULL) { + sqlite3DbFree(db, opts_buff); + goto cleanup; + } - sqlite3VdbeAddOp4(v, - OP_String8, 0, first_col, 0, + sqlite3VdbeAddOp4(v, OP_String8, 0, first_col, 0, trigger_name, P4_DYNAMIC); sqlite3VdbeAddOp2(v, OP_Integer, trigger->space_id, first_col + 1); @@ -263,11 +261,7 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list, trigger = NULL; } - triggerfinish_cleanup: - if (db->mallocFailed) { - sqlite3DbFree(db, sql_str); - sqlite3DbFree(db, opts_buff); - } +cleanup: sql_trigger_delete(db, trigger); assert(parse->parsed_ast.trigger == NULL || parse->parse_only); sqlite3DeleteTriggerStep(db, step_list);