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 3EEA323E7E for ; Wed, 11 Sep 2019 09:10:58 -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 FZaXaJDGQk7s for ; Wed, 11 Sep 2019 09:10:58 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 2C8C323268 for ; Wed, 11 Sep 2019 09:10:57 -0400 (EDT) From: Ilya Kosarev Subject: [tarantool-patches] [PATCH v3 2/5] refactoring: remove exceptions from used in alter.cc outer functions Date: Wed, 11 Sep 2019 16:05:16 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: 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 Cc: georgy@tarantool.org, i.kosarev@corp.mail.ru, Ilya Kosarev Outer functions, used in alter.cc, are cleared from exceptions. Their usage in alter.cc is updated. Most obvious exceptions in alter.cc are refactored. --- src/box/alter.cc | 652 ++++++++++++++++++++++++++++------------- src/box/replication.cc | 53 ++-- src/box/replication.h | 2 +- src/box/schema.cc | 28 +- src/box/schema.h | 13 +- src/box/sequence.h | 9 - src/box/user.cc | 9 +- src/box/user.h | 14 +- 8 files changed, 506 insertions(+), 274 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 522b1b596..0a9cc5751 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -100,7 +100,9 @@ access_check_ddl(const char *name, uint32_t object_id, uint32_t owner_uid, return; /* Access granted. */ } /* Create a meaningful error message. */ - struct user *user = user_find_xc(cr->uid); + struct user *user = user_find(cr->uid); + if (user == NULL) + diag_raise(); const char *object_name; const char *pname; if (access & PRIV_U) { @@ -1910,8 +1912,10 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) * old_tuple ID field, if old_tuple is set, since UPDATE * may have changed space id. */ - uint32_t old_id = tuple_field_u32_xc(old_tuple ? old_tuple : new_tuple, - BOX_SPACE_FIELD_ID); + uint32_t old_id; + if (tuple_field_u32(old_tuple ? old_tuple : new_tuple, + BOX_SPACE_FIELD_ID, &old_id) != 0) + return -1; struct space *old_space = space_by_id(old_id); if (new_tuple != NULL && old_space == NULL) { /* INSERT */ struct space_def *def = @@ -1946,14 +1950,16 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) */ struct trigger *on_rollback = txn_alter_trigger_new(on_create_space_rollback, space); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); if (def->opts.is_view) { struct Select *select = sql_view_compile(sql_get(), - def->opts.sql); + def->opts.sql); if (select == NULL) - diag_raise(); + return -1; auto select_guard = make_scoped_guard([=] { - sql_select_delete(sql_get(), select); + sql_select_delete(sql_get(), select); }); const char *disappeared_space; if (update_view_references(select, 1, false, @@ -1964,16 +1970,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) */ update_view_references(select, -1, false, &disappeared_space); - tnt_raise(ClientError, ER_NO_SUCH_SPACE, + diag_set(ClientError, ER_NO_SUCH_SPACE, disappeared_space); + return -1; } struct trigger *on_commit_view = txn_alter_trigger_new(on_create_view_commit, select); + if (on_commit_view == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit_view); struct trigger *on_rollback_view = txn_alter_trigger_new(on_create_view_rollback, select); + if (on_rollback_view == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback_view); select_guard.is_active = false; } @@ -1982,23 +1993,30 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) old_space->def->uid, SC_SPACE, PRIV_D); /* Verify that the space is empty (has no indexes) */ if (old_space->index_count) { - tnt_raise(ClientError, ER_DROP_SPACE, + diag_set(ClientError, ER_DROP_SPACE, space_name(old_space), "the space has indexes"); + return -1; } - if (schema_find_grants("space", old_space->def->id)) { - tnt_raise(ClientError, ER_DROP_SPACE, - space_name(old_space), - "the space has grants"); + bool out; + if (schema_find_grants("space", old_space->def->id, &out) != 0) { + return -1; + } + if (out) { + diag_set(ClientError, ER_DROP_SPACE, + space_name(old_space), + "the space has grants"); + return -1; } if (space_has_data(BOX_TRUNCATE_ID, 0, old_space->def->id)) tnt_raise(ClientError, ER_DROP_SPACE, space_name(old_space), "the space has truncate record"); if (old_space->def->view_ref_count > 0) { - tnt_raise(ClientError, ER_DROP_SPACE, + diag_set(ClientError, ER_DROP_SPACE, space_name(old_space), "other views depend on this space"); + return -1; } /* * No need to check existence of parent keys, @@ -2007,15 +2025,17 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) * one referenced index which can't be dropped * before constraint itself. */ - if (! rlist_empty(&old_space->child_fk_constraint)) { - tnt_raise(ClientError, ER_DROP_SPACE, + if (!rlist_empty(&old_space->child_fk_constraint)) { + diag_set(ClientError, ER_DROP_SPACE, space_name(old_space), "the space has foreign key constraints"); + return -1; } if (!rlist_empty(&old_space->ck_constraint)) { - tnt_raise(ClientError, ER_DROP_SPACE, + diag_set(ClientError, ER_DROP_SPACE, space_name(old_space), "the space has check constraints"); + return -1; } /** * The space must be deleted from the space @@ -2031,26 +2051,34 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) ++schema_version; struct trigger *on_commit = txn_alter_trigger_new(on_drop_space_commit, old_space); + if (on_commit == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit); struct trigger *on_rollback = txn_alter_trigger_new(on_drop_space_rollback, old_space); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); if (old_space->def->opts.is_view) { struct Select *select = sql_view_compile(sql_get(), old_space->def->opts.sql); if (select == NULL) - diag_raise(); + return -1; auto select_guard = make_scoped_guard([=] { - sql_select_delete(sql_get(), select); + sql_select_delete(sql_get(), select); }); struct trigger *on_commit_view = txn_alter_trigger_new(on_drop_view_commit, select); + if (on_commit_view == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit_view); struct trigger *on_rollback_view = txn_alter_trigger_new(on_drop_view_rollback, select); + if (on_rollback_view == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback_view); update_view_references(select, -1, true, NULL); select_guard.is_active = false; @@ -2065,28 +2093,39 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) access_check_ddl(def->name, def->id, def->uid, SC_SPACE, PRIV_A); if (def->id != space_id(old_space)) - tnt_raise(ClientError, ER_ALTER_SPACE, + if (def->id != space_id(old_space)) { + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "space id is immutable"); - if (strcmp(def->engine_name, old_space->def->engine_name) != 0) - tnt_raise(ClientError, ER_ALTER_SPACE, + return -1; + } + if (strcmp(def->engine_name, old_space->def->engine_name) != 0) { + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not change space engine"); - if (def->opts.group_id != space_group_id(old_space)) - tnt_raise(ClientError, ER_ALTER_SPACE, + return -1; + } + if (def->opts.group_id != space_group_id(old_space)) { + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "replication group is immutable"); - if (def->opts.is_view != old_space->def->opts.is_view) - tnt_raise(ClientError, ER_ALTER_SPACE, + return -1; + } + if (def->opts.is_view != old_space->def->opts.is_view) { + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not convert a space to " "a view and vice versa"); + return -1; + } if (strcmp(def->name, old_space->def->name) != 0 && - old_space->def->view_ref_count > 0) - tnt_raise(ClientError, ER_ALTER_SPACE, - space_name(old_space), - "can not rename space which is referenced by " - "view"); + old_space->def->view_ref_count > 0) { + diag_set(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not rename space which is referenced by " + "view"); + return -1; + } /* * Allow change of space properties, but do it * in WAL-error-safe mode. @@ -2198,8 +2237,9 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) BOX_INDEX_FIELD_ID); struct space *old_space = space_cache_find_xc(id); if (old_space->def->opts.is_view) { - tnt_raise(ClientError, ER_ALTER_SPACE, space_name(old_space), + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not add index on a view"); + return -1; } enum priv_type priv_type = new_tuple ? PRIV_C : PRIV_D; if (old_tuple && new_tuple) @@ -2215,24 +2255,28 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) /* * Dropping the primary key in a system space: off limits. */ - if (space_is_system(old_space)) - tnt_raise(ClientError, ER_LAST_DROP, + if (space_is_system(old_space)) { + diag_set(ClientError, ER_LAST_DROP, space_name(old_space)); + return -1; + } /* * Can't drop primary key before secondary keys. */ if (old_space->index_count > 1) { - tnt_raise(ClientError, ER_DROP_PRIMARY_KEY, + diag_set(ClientError, ER_DROP_PRIMARY_KEY, space_name(old_space)); + return -1; } /* * Can't drop primary key before space sequence. */ if (old_space->sequence != NULL) { - tnt_raise(ClientError, ER_ALTER_SPACE, + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not drop primary key while " "space sequence exists"); + return -1; } } @@ -2241,9 +2285,10 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * A secondary index can not be created without * a primary key. */ - tnt_raise(ClientError, ER_ALTER_SPACE, + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not add a secondary key before primary"); + return -1; } struct alter_space *alter = alter_space_new(old_space); @@ -2265,9 +2310,10 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) */ if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint, iid)) { - tnt_raise(ClientError, ER_ALTER_SPACE, + diag_set(ClientError, ER_ALTER_SPACE, space_name(old_space), "can not drop a referenced index"); + return -1; } alter_space_move_indexes(alter, 0, iid); (void) new DropIndex(alter, old_index); @@ -2331,9 +2377,10 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) index_def)) { if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint, iid)) { - tnt_raise(ClientError, ER_ALTER_SPACE, - space_name(old_space), - "can not alter a referenced index"); + diag_set(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not alter a referenced index"); + return -1; } /* * Operation demands an index rebuild. @@ -2388,9 +2435,12 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) return 0; } - uint32_t space_id = - tuple_field_u32_xc(new_tuple, BOX_TRUNCATE_FIELD_SPACE_ID); - struct space *old_space = space_cache_find_xc(space_id); + uint32_t space_id; + if (tuple_field_u32(new_tuple, BOX_TRUNCATE_FIELD_SPACE_ID, &space_id) != 0) + return -1; + struct space *old_space = space_cache_find(space_id); + if (old_space == NULL) + return -1; if (stmt->row->type == IPROTO_INSERT) { /* @@ -2405,9 +2455,11 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) * with internal objects. Since space truncation doesn't * invoke triggers, we don't permit it for system spaces. */ - if (space_is_system(old_space)) - tnt_raise(ClientError, ER_TRUNCATE_SYSTEM_SPACE, + if (space_is_system(old_space)) { + diag_set(ClientError, ER_TRUNCATE_SYSTEM_SPACE, space_name(old_space)); + return -1; + } /* * Check if a write privilege was given, raise an error if not. @@ -2588,7 +2640,9 @@ static int user_cache_remove_user(struct trigger *trigger, void * /* event */) { struct tuple *tuple = (struct tuple *)trigger->data; - uint32_t uid = tuple_field_u32_xc(tuple, BOX_USER_FIELD_ID); + uint32_t uid; + if (tuple_field_u32(tuple, BOX_USER_FIELD_ID, &uid) != 0) + return -1; user_cache_delete(uid); return 0; } @@ -2616,8 +2670,10 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) struct tuple *old_tuple = stmt->old_tuple; struct tuple *new_tuple = stmt->new_tuple; - uint32_t uid = tuple_field_u32_xc(old_tuple ? old_tuple : new_tuple, - BOX_USER_FIELD_ID); + uint32_t uid; + if (tuple_field_u32(old_tuple ? old_tuple : new_tuple, + BOX_USER_FIELD_ID, &uid) != 0) + return -1; struct user *old_user = user_by_id(uid); if (new_tuple != NULL && old_user == NULL) { /* INSERT */ struct user_def *user = user_def_new_from_tuple(new_tuple); @@ -2628,6 +2684,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) def_guard.is_active = false; struct trigger *on_rollback = txn_alter_trigger_new(user_cache_remove_user, new_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } else if (new_tuple == NULL) { /* DELETE */ access_check_ddl(old_user->def->name, old_user->def->uid, @@ -2635,9 +2693,10 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) PRIV_D); /* Can't drop guest or super user */ if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) { - tnt_raise(ClientError, ER_DROP_USER, + diag_set(ClientError, ER_DROP_USER, old_user->def->name, "the user or the role is a system"); + return -1; } /* * Can only delete user if it has no spaces, @@ -2650,6 +2709,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) user_cache_delete(uid); struct trigger *on_rollback = txn_alter_trigger_new(user_cache_alter_user, old_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } else { /* UPDATE, REPLACE */ assert(old_user != NULL && new_tuple != NULL); @@ -2666,6 +2727,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) def_guard.is_active = false; struct trigger *on_rollback = txn_alter_trigger_new(user_cache_alter_user, old_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } return 0; @@ -2678,11 +2741,12 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) * @param[out] fid Function identifier. * @param[out] uid Owner identifier. */ -static inline void +static inline int func_def_get_ids_from_tuple(struct tuple *tuple, uint32_t *fid, uint32_t *uid) { - *fid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_ID); - *uid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_UID); + if (tuple_field_u32(tuple, BOX_FUNC_FIELD_ID, fid) != 0) + return -1; + return tuple_field_u32(tuple, BOX_FUNC_FIELD_UID, uid); } /** Create a function definition from tuple. */ @@ -2738,7 +2802,8 @@ func_def_new_from_tuple(struct tuple *tuple) if (def == NULL) tnt_raise(OutOfMemory, def_sz, "malloc", "def"); auto def_guard = make_scoped_guard([=] { free(def); }); - func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid); + if (func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid) != 0) + return NULL; if (def->fid > BOX_FUNCTION_MAX) { tnt_raise(ClientError, ER_CREATE_FUNCTION, tt_cstr(name, name_len), "function id is too big"); @@ -2905,8 +2970,10 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) struct tuple *old_tuple = stmt->old_tuple; struct tuple *new_tuple = stmt->new_tuple; - uint32_t fid = tuple_field_u32_xc(old_tuple ? old_tuple : new_tuple, - BOX_FUNC_FIELD_ID); + uint32_t fid; + if (tuple_field_u32(old_tuple ? old_tuple : new_tuple, + BOX_FUNC_FIELD_ID, &fid) != 0) + return -1; struct func *old_func = func_by_id(fid); if (new_tuple != NULL && old_func == NULL) { /* INSERT */ struct func_def *def = func_def_new_from_tuple(new_tuple); @@ -2915,9 +2982,11 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) PRIV_C); struct trigger *on_rollback = txn_alter_trigger_new(on_create_func_rollback, NULL); + if (on_rollback == NULL) + return -1; struct func *func = func_new(def); if (func == NULL) - diag_raise(); + return -1; def_guard.is_active = false; func_cache_insert(func); on_rollback->data = func; @@ -2925,7 +2994,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) trigger_run_xc(&on_alter_func, func); } else if (new_tuple == NULL) { /* DELETE */ uint32_t uid; - func_def_get_ids_from_tuple(old_tuple, &fid, &uid); + if (func_def_get_ids_from_tuple(old_tuple, &fid, &uid) != 0) + return -1; /* * Can only delete func if you're the one * who created it or a superuser. @@ -2933,10 +3003,15 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) access_check_ddl(old_func->def->name, fid, uid, SC_FUNCTION, PRIV_D); /* Can only delete func if it has no grants. */ - if (schema_find_grants("function", old_func->def->fid)) { - tnt_raise(ClientError, ER_DROP_FUNCTION, - (unsigned) old_func->def->uid, - "function has grants"); + bool out; + if (schema_find_grants("function", old_func->def->fid, &out) != 0) { + return -1; + } + if (out) { + diag_set(ClientError, ER_DROP_FUNCTION, + (unsigned) old_func->def->uid, + "function has grants"); + return -1; } if (old_func != NULL && space_has_data(BOX_FUNC_INDEX_ID, 1, old_func->def->fid)) { @@ -2954,6 +3029,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(on_drop_func_commit, old_func); struct trigger *on_rollback = txn_alter_trigger_new(on_drop_func_rollback, old_func); + if (on_commit == NULL || on_rollback == NULL) + return -1; func_cache_delete(old_func->def->fid); txn_stmt_on_commit(stmt, on_commit); txn_stmt_on_rollback(stmt, on_rollback); @@ -2971,9 +3048,12 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) }); old_def = func_def_new_from_tuple(old_tuple); new_def = func_def_new_from_tuple(new_tuple); + if (old_def == NULL || new_def == NULL) + return -1; if (func_def_cmp(new_def, old_def) != 0) { - tnt_raise(ClientError, ER_UNSUPPORTED, "function", + diag_set(ClientError, ER_UNSUPPORTED, "function", "alter"); + return -1; } } return 0; @@ -3116,12 +3196,16 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(on_drop_collation_commit, NULL); struct trigger *on_rollback = txn_alter_trigger_new(on_drop_collation_rollback, NULL); + if (on_commit == NULL || on_rollback == NULL) + return -1; /* * TODO: Check that no index uses the collation * identifier. */ - int32_t old_id = tuple_field_u32_xc(old_tuple, - BOX_COLLATION_FIELD_ID); + uint32_t out; + if (tuple_field_u32(old_tuple, BOX_COLLATION_FIELD_ID, &out) != 0) + return -1; + int32_t old_id = out; /* * Don't allow user to drop "none" collation * since it is very special and vastly used @@ -3129,8 +3213,9 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) * fact that "none" collation features id == 0. */ if (old_id == COLL_NONE) { - tnt_raise(ClientError, ER_DROP_COLLATION, "none", + diag_set(ClientError, ER_DROP_COLLATION, "none", "system collation"); + return -1; } struct coll_id *old_coll_id = coll_by_id(old_id); assert(old_coll_id != NULL); @@ -3151,17 +3236,19 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) /* INSERT */ struct trigger *on_rollback = txn_alter_trigger_new(on_create_collation_rollback, NULL); + if (on_rollback == NULL) + return -1; struct coll_id_def new_def; coll_id_def_new_from_tuple(new_tuple, &new_def); access_check_ddl(new_def.name, new_def.id, new_def.owner_id, SC_COLLATION, PRIV_C); struct coll_id *new_coll_id = coll_id_new(&new_def); if (new_coll_id == NULL) - diag_raise(); + return -1; struct coll_id *replaced_id; if (coll_id_cache_replace(new_coll_id, &replaced_id) != 0) { coll_id_delete(new_coll_id); - diag_raise(); + return -1; } assert(replaced_id == NULL); on_rollback->data = new_coll_id; @@ -3169,7 +3256,8 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) } else { /* UPDATE */ assert(new_tuple != NULL && old_tuple != NULL); - tnt_raise(ClientError, ER_UNSUPPORTED, "collation", "alter"); + diag_set(ClientError, ER_UNSUPPORTED, "collation", "alter"); + return -1; } return 0; } @@ -3177,20 +3265,25 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) /** * Create a privilege definition from tuple. */ -void +int priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) { - priv->grantor_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ID); - priv->grantee_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_UID); + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ID, &(priv->grantor_id)) != 0) + return -1; + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &(priv->grantee_id)) != 0) + return -1; const char *object_type = - tuple_field_cstr_xc(tuple, BOX_PRIV_FIELD_OBJECT_TYPE); + tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE); + if (object_type == NULL) + return -1; priv->object_type = schema_object_type(object_type); const char *data = tuple_field(tuple, BOX_PRIV_FIELD_OBJECT_ID); if (data == NULL) { - tnt_raise(ClientError, ER_NO_SUCH_FIELD_NO, + diag_set(ClientError, ER_NO_SUCH_FIELD_NO, BOX_PRIV_FIELD_OBJECT_ID + TUPLE_INDEX_BASE); + return -1; } /* * When granting or revoking privileges on a whole entity @@ -3208,14 +3301,19 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) } FALLTHROUGH; default: - priv->object_id = tuple_field_u32_xc(tuple, - BOX_PRIV_FIELD_OBJECT_ID); + if (tuple_field_u32(tuple,BOX_PRIV_FIELD_OBJECT_ID, &(priv->object_id)) != 0) + return -1; } if (priv->object_type == SC_UNKNOWN) { - tnt_raise(ClientError, ER_UNKNOWN_SCHEMA_OBJECT, + diag_set(ClientError, ER_UNKNOWN_SCHEMA_OBJECT, object_type); + return -1; } - priv->access = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ACCESS); + uint32_t out; + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ACCESS, &out) != 0) + return -1; + priv->access = out; + return 0; } /* @@ -3231,7 +3329,9 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) static void priv_def_check(struct priv_def *priv, enum priv_type priv_type) { - struct user *grantor = user_find_xc(priv->grantor_id); + struct user *grantor = user_find(priv->grantor_id); + if (grantor == NULL) + diag_raise(); /* May be a role */ struct user *grantee = user_by_id(priv->grantee_id); if (grantee == NULL) { @@ -3265,7 +3365,11 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) } case SC_FUNCTION: { - struct func *func = func_cache_find(priv->object_id); + struct func *func = func_by_id(priv->object_id); + if (func == NULL) { + diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(priv->object_id)); + diag_raise(); + } if (func->def->uid != grantor->def->uid && grantor->def->uid != ADMIN) { tnt_raise(AccessDeniedError, @@ -3308,7 +3412,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) grantor->def->name); } /* Not necessary to do during revoke, but who cares. */ - role_check(grantee, role); + if (role_check(grantee, role) != 0) + diag_raise(); break; } case SC_USER: @@ -3384,7 +3489,8 @@ revoke_priv(struct trigger *trigger, void *event) (void) event; struct tuple *tuple = (struct tuple *)trigger->data; struct priv_def priv; - priv_def_create_from_tuple(&priv, tuple); + if (priv_def_create_from_tuple(&priv, tuple) != 0) + return -1; priv.access = 0; grant_or_revoke(&priv); return 0; @@ -3397,7 +3503,8 @@ modify_priv(struct trigger *trigger, void *event) (void) event; struct tuple *tuple = (struct tuple *)trigger->data; struct priv_def priv; - priv_def_create_from_tuple(&priv, tuple); + if (priv_def_create_from_tuple(&priv, tuple) != 0) + return -1; grant_or_revoke(&priv); return 0; } @@ -3416,27 +3523,36 @@ on_replace_dd_priv(struct trigger * /* trigger */, void *event) struct priv_def priv; if (new_tuple != NULL && old_tuple == NULL) { /* grant */ - priv_def_create_from_tuple(&priv, new_tuple); + if (priv_def_create_from_tuple(&priv, new_tuple) != 0) + return -1; priv_def_check(&priv, PRIV_GRANT); grant_or_revoke(&priv); struct trigger *on_rollback = txn_alter_trigger_new(revoke_priv, new_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } else if (new_tuple == NULL) { /* revoke */ assert(old_tuple); - priv_def_create_from_tuple(&priv, old_tuple); + if (priv_def_create_from_tuple(&priv, old_tuple) != 0) + return -1; priv_def_check(&priv, PRIV_REVOKE); priv.access = 0; grant_or_revoke(&priv); struct trigger *on_rollback = txn_alter_trigger_new(modify_priv, old_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } else { /* modify */ - priv_def_create_from_tuple(&priv, new_tuple); + if (priv_def_create_from_tuple(&priv, new_tuple) != 0) + return -1; priv_def_check(&priv, PRIV_GRANT); grant_or_revoke(&priv); struct trigger *on_rollback = txn_alter_trigger_new(modify_priv, old_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); } return 0; @@ -3462,13 +3578,18 @@ on_replace_dd_schema(struct trigger * /* trigger */, void *event) struct txn_stmt *stmt = txn_current_stmt(txn); struct tuple *old_tuple = stmt->old_tuple; struct tuple *new_tuple = stmt->new_tuple; - const char *key = tuple_field_cstr_xc(new_tuple ? new_tuple : old_tuple, + const char *key = tuple_field_cstr(new_tuple ? new_tuple : old_tuple, BOX_SCHEMA_FIELD_KEY); + if (key == NULL) + return -1; if (strcmp(key, "cluster") == 0) { - if (new_tuple == NULL) - tnt_raise(ClientError, ER_REPLICASET_UUID_IS_RO); + if (new_tuple == NULL) { + diag_set(ClientError, ER_REPLICASET_UUID_IS_RO); + return -1; + } tt_uuid uu; - tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu); + if (tuple_field_uuid(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu) != 0) + return -1; REPLICASET_UUID = uu; say_info("cluster uuid %s", tt_uuid_str(&uu)); } @@ -3484,10 +3605,12 @@ static int register_replica(struct trigger *trigger, void * /* event */) { struct tuple *new_tuple = (struct tuple *)trigger->data; - - uint32_t id = tuple_field_u32_xc(new_tuple, BOX_CLUSTER_FIELD_ID); + uint32_t id; + if (tuple_field_u32(new_tuple, BOX_CLUSTER_FIELD_ID, &id) != 0) + return -1; tt_uuid uuid; - tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, &uuid); + if (tuple_field_uuid(new_tuple, BOX_CLUSTER_FIELD_UUID, &uuid) != 0) + return -1; struct replica *replica = replica_by_uuid(&uuid); if (replica != NULL) { replica_set_id(replica, id); @@ -3495,7 +3618,7 @@ register_replica(struct trigger *trigger, void * /* event */) try { replica = replicaset_add(id, &uuid); /* Can't throw exceptions from on_commit trigger */ - } catch(Exception *e) { + } catch (Exception *e) { panic("Can't register replica: %s", e->errmsg); } } @@ -3508,7 +3631,9 @@ unregister_replica(struct trigger *trigger, void * /* event */) struct tuple *old_tuple = (struct tuple *)trigger->data; struct tt_uuid old_uuid; - tuple_field_uuid_xc(old_tuple, BOX_CLUSTER_FIELD_UUID, &old_uuid); + + if (tuple_field_uuid(old_tuple, BOX_CLUSTER_FIELD_UUID, &old_uuid) != 0) + return -1; struct replica *replica = replica_by_uuid(&old_uuid); assert(replica != NULL); @@ -3543,16 +3668,21 @@ on_replace_dd_cluster(struct trigger *trigger, void *event) struct tuple *old_tuple = stmt->old_tuple; struct tuple *new_tuple = stmt->new_tuple; if (new_tuple != NULL) { /* Insert or replace */ - /* Check fields */ - uint32_t replica_id = - tuple_field_u32_xc(new_tuple, BOX_CLUSTER_FIELD_ID); - replica_check_id(replica_id); tt_uuid replica_uuid; - tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, - &replica_uuid); - if (tt_uuid_is_nil(&replica_uuid)) - tnt_raise(ClientError, ER_INVALID_UUID, + /* Check fields */ + uint32_t replica_id; + if (tuple_field_u32(new_tuple, BOX_CLUSTER_FIELD_ID, &replica_id) != 0) + return -1; + if (replica_check_id(replica_id) != 0) + return -1; + if (tuple_field_uuid(new_tuple, BOX_CLUSTER_FIELD_UUID, + &replica_uuid) != 0) + return -1; + if (tt_uuid_is_nil(&replica_uuid)) { + diag_set(ClientError, ER_INVALID_UUID, tt_uuid_str(&replica_uuid)); + return -1; + } if (old_tuple != NULL) { /* * Forbid changes of UUID for a registered instance: @@ -3560,17 +3690,21 @@ on_replace_dd_cluster(struct trigger *trigger, void *event) * in sync with appliers and relays. */ tt_uuid old_uuid; - tuple_field_uuid_xc(old_tuple, BOX_CLUSTER_FIELD_UUID, - &old_uuid); + if (tuple_field_uuid(old_tuple, BOX_CLUSTER_FIELD_UUID, + &old_uuid) != 0) + return -1; if (!tt_uuid_is_equal(&replica_uuid, &old_uuid)) { - tnt_raise(ClientError, ER_UNSUPPORTED, + diag_set(ClientError, ER_UNSUPPORTED, "Space _cluster", "updates of instance uuid"); + return -1; } } else { struct trigger *on_commit; on_commit = txn_alter_trigger_new(register_replica, new_tuple); + if (on_commit == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit); } } else { @@ -3579,13 +3713,17 @@ on_replace_dd_cluster(struct trigger *trigger, void *event) * from _cluster. */ assert(old_tuple != NULL); - uint32_t replica_id = - tuple_field_u32_xc(old_tuple, BOX_CLUSTER_FIELD_ID); - replica_check_id(replica_id); + uint32_t replica_id; + if (tuple_field_u32(old_tuple, BOX_CLUSTER_FIELD_ID, &replica_id) != 0) + return -1; + if (replica_check_id(replica_id) != 0) + return -1; struct trigger *on_commit; on_commit = txn_alter_trigger_new(unregister_replica, old_tuple); + if (on_commit == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit); } return 0; @@ -3712,30 +3850,43 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) SC_SEQUENCE, PRIV_C); struct trigger *on_rollback = txn_alter_trigger_new(on_create_sequence_rollback, NULL); - seq = sequence_new_xc(new_def); + if (on_rollback == NULL) + return -1; + seq = sequence_new(new_def); + if (seq == NULL) + return -1; sequence_cache_insert(seq); on_rollback->data = seq; txn_stmt_on_rollback(stmt, on_rollback); } else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */ - uint32_t id = tuple_field_u32_xc(old_tuple, - BOX_SEQUENCE_DATA_FIELD_ID); + uint32_t id; + if (tuple_field_u32(old_tuple,BOX_SEQUENCE_DATA_FIELD_ID, &id) != 0) + return -1; seq = sequence_by_id(id); assert(seq != NULL); access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, SC_SEQUENCE, PRIV_D); + bool out; if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id)) tnt_raise(ClientError, ER_DROP_SEQUENCE, seq->def->name, "the sequence has data"); if (space_has_data(BOX_SPACE_SEQUENCE_ID, 1, id)) tnt_raise(ClientError, ER_DROP_SEQUENCE, seq->def->name, "the sequence is in use"); - if (schema_find_grants("sequence", seq->def->id)) - tnt_raise(ClientError, ER_DROP_SEQUENCE, + if (schema_find_grants("sequence", seq->def->id, &out) != 0) { + return -1; + } + if (out) { + diag_set(ClientError, ER_DROP_SEQUENCE, seq->def->name, "the sequence has grants"); + return -1; + } struct trigger *on_commit = txn_alter_trigger_new(on_drop_sequence_commit, seq); struct trigger *on_rollback = txn_alter_trigger_new(on_drop_sequence_rollback, seq); + if (on_commit == NULL || on_rollback == NULL) + return -1; sequence_cache_delete(seq->def->id); txn_stmt_on_commit(stmt, on_commit); txn_stmt_on_rollback(stmt, on_rollback); @@ -3750,6 +3901,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(on_alter_sequence_commit, seq->def); struct trigger *on_rollback = txn_alter_trigger_new(on_alter_sequence_rollback, seq->def); + if (on_commit == NULL || on_rollback == NULL) + return -1; seq->def = new_def; txn_stmt_on_commit(stmt, on_commit); txn_stmt_on_rollback(stmt, on_rollback); @@ -3765,9 +3918,12 @@ static int on_drop_sequence_data_rollback(struct trigger *trigger, void * /* event */) { struct tuple *tuple = (struct tuple *)trigger->data; - uint32_t id = tuple_field_u32_xc(tuple, BOX_SEQUENCE_DATA_FIELD_ID); - int64_t val = tuple_field_i64_xc(tuple, BOX_SEQUENCE_DATA_FIELD_VALUE); - + uint32_t id; + if (tuple_field_u32(tuple, BOX_SEQUENCE_DATA_FIELD_ID, &id) != 0) + return -1; + int64_t val; + if (tuple_field_i64(tuple, BOX_SEQUENCE_DATA_FIELD_VALUE, &val) != 0) + return -1; struct sequence *seq = sequence_by_id(id); assert(seq != NULL); if (sequence_set(seq, val) != 0) @@ -3787,17 +3943,21 @@ on_replace_dd_sequence_data(struct trigger * /* trigger */, void *event) struct tuple *old_tuple = stmt->old_tuple; struct tuple *new_tuple = stmt->new_tuple; - uint32_t id = tuple_field_u32_xc(old_tuple ?: new_tuple, - BOX_SEQUENCE_DATA_FIELD_ID); - struct sequence *seq = sequence_cache_find(id); + uint32_t id; + if (tuple_field_u32(old_tuple ?: new_tuple,BOX_SEQUENCE_DATA_FIELD_ID, + &id) != 0) + return -1; + struct sequence *seq = sequence_by_id(id); if (seq == NULL) - diag_raise(); - if (new_tuple != NULL) { /* INSERT, UPDATE */ - int64_t value = tuple_field_i64_xc(new_tuple, - BOX_SEQUENCE_DATA_FIELD_VALUE); + return -1; + if (new_tuple != NULL) { /* INSERT, UPDATE */ + int64_t value; + if (tuple_field_i64(new_tuple, BOX_SEQUENCE_DATA_FIELD_VALUE, + &value) != 0) + return -1; if (sequence_set(seq, value) != 0) - diag_raise(); - } else { /* DELETE */ + return -1; + } else { /* DELETE */ /* * A sequence isn't supposed to roll back to the old * value if the transaction it was used in is aborted @@ -3806,7 +3966,9 @@ on_replace_dd_sequence_data(struct trigger * /* trigger */, void *event) * on rollback. */ struct trigger *on_rollback = txn_alter_trigger_new( - on_drop_sequence_data_rollback, old_tuple); + on_drop_sequence_data_rollback, old_tuple); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); sequence_reset(seq); } @@ -3856,12 +4018,16 @@ static int set_space_sequence(struct trigger *trigger, void * /* event */) { struct tuple *tuple = (struct tuple *)trigger->data; - uint32_t space_id = tuple_field_u32_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_ID); - uint32_t sequence_id = tuple_field_u32_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_SEQUENCE_ID); - bool is_generated = tuple_field_bool_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_IS_GENERATED); + uint32_t space_id; + if (tuple_field_u32(tuple, BOX_SPACE_SEQUENCE_FIELD_ID, &space_id) != 0) + return -1; + uint32_t sequence_id; + if (tuple_field_u32(tuple, BOX_SPACE_SEQUENCE_FIELD_SEQUENCE_ID, &sequence_id) != 0) + return -1; + bool is_generated; + if (tuple_field_bool(tuple, BOX_SPACE_SEQUENCE_FIELD_IS_GENERATED, + &is_generated) != 0) + return -1; struct space *space = space_by_id(space_id); assert(space != NULL); struct sequence *seq = sequence_by_id(sequence_id); @@ -3882,8 +4048,9 @@ static int clear_space_sequence(struct trigger *trigger, void * /* event */) { struct tuple *tuple = (struct tuple *)trigger->data; - uint32_t space_id = tuple_field_u32_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_ID); + uint32_t space_id; + if (tuple_field_u32(tuple, BOX_SPACE_SEQUENCE_FIELD_ID, &space_id) != 0) + return -1; struct space *space = space_by_id(space_id); assert(space != NULL); assert(space->sequence != NULL); @@ -3906,16 +4073,22 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) struct txn *txn = (struct txn *) event; struct txn_stmt *stmt = txn_current_stmt(txn); struct tuple *tuple = stmt->new_tuple ? stmt->new_tuple : stmt->old_tuple; - - uint32_t space_id = tuple_field_u32_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_ID); - uint32_t sequence_id = tuple_field_u32_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_SEQUENCE_ID); - bool is_generated = tuple_field_bool_xc(tuple, - BOX_SPACE_SEQUENCE_FIELD_IS_GENERATED); - - struct space *space = space_cache_find_xc(space_id); - struct sequence *seq = sequence_cache_find(sequence_id); + uint32_t space_id; + if (tuple_field_u32(tuple,BOX_SPACE_SEQUENCE_FIELD_ID, &space_id) != 0) + return -1; + uint32_t sequence_id; + if (tuple_field_u32(tuple, BOX_SPACE_SEQUENCE_FIELD_SEQUENCE_ID, &sequence_id) != 0) + return -1; + bool is_generated; + if (tuple_field_bool(tuple, BOX_SPACE_SEQUENCE_FIELD_IS_GENERATED, + &is_generated) != 0) + return -1; + struct space *space = space_cache_find(space_id); + if (space == NULL) + return -1; + struct sequence *seq = sequence_by_id(sequence_id); + if (seq == NULL) + return -1; enum priv_type priv_type = stmt->new_tuple ? PRIV_C : PRIV_D; if (stmt->new_tuple && stmt->old_tuple) @@ -3948,9 +4121,10 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) free(sequence_path); }); if (seq->is_generated) { - tnt_raise(ClientError, ER_ALTER_SPACE, + diag_set(ClientError, ER_ALTER_SPACE, space_name(space), "can not attach generated sequence"); + return -1; } struct trigger *on_rollback; if (stmt->old_tuple != NULL) @@ -3959,6 +4133,8 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) else on_rollback = txn_alter_trigger_new(clear_space_sequence, stmt->new_tuple); + if (on_rollback == NULL) + return -1; seq->is_generated = is_generated; space->sequence = seq; space->sequence_fieldno = sequence_fieldno; @@ -3970,6 +4146,8 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) struct trigger *on_rollback; on_rollback = txn_alter_trigger_new(set_space_sequence, stmt->old_tuple); + if (on_rollback == NULL) + return -1; assert(space->sequence == seq); seq->is_generated = false; space->sequence = NULL; @@ -4060,19 +4238,24 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) struct trigger *on_rollback = txn_alter_trigger_new(NULL, NULL); struct trigger *on_commit = txn_alter_trigger_new(on_replace_trigger_commit, NULL); + if (on_rollback == NULL || on_commit == NULL) + return -1; if (old_tuple != NULL && new_tuple == NULL) { /* DROP trigger. */ uint32_t trigger_name_len; - const char *trigger_name_src = - tuple_field_str_xc(old_tuple, BOX_TRIGGER_FIELD_NAME, - &trigger_name_len); - uint32_t space_id = - tuple_field_u32_xc(old_tuple, - BOX_TRIGGER_FIELD_SPACE_ID); - char *trigger_name = - (char *)region_alloc_xc(&fiber()->gc, - trigger_name_len + 1); + const char *trigger_name_src = tuple_field_str(old_tuple, + BOX_TRIGGER_FIELD_NAME, &trigger_name_len); + if (trigger_name_src == NULL) + return -1; + uint32_t space_id; + if (tuple_field_u32(old_tuple,BOX_TRIGGER_FIELD_SPACE_ID, + &space_id) != 0) + return -1; + char *trigger_name = (char *)region_alloc(&fiber()->gc, + trigger_name_len + 1); + if (trigger_name == NULL) + return -1; memcpy(trigger_name, trigger_name_src, trigger_name_len); trigger_name[trigger_name_len] = 0; @@ -4088,21 +4271,21 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) } else { /* INSERT, REPLACE trigger. */ uint32_t trigger_name_len; - const char *trigger_name_src = - tuple_field_str_xc(new_tuple, BOX_TRIGGER_FIELD_NAME, - &trigger_name_len); - - const char *space_opts = - tuple_field_with_type_xc(new_tuple, - BOX_TRIGGER_FIELD_OPTS, - MP_MAP); + const char *trigger_name_src = tuple_field_str(new_tuple, + BOX_TRIGGER_FIELD_NAME, &trigger_name_len); + if (trigger_name_src == NULL) + return -1; + const char *space_opts = tuple_field_with_type(new_tuple, + BOX_TRIGGER_FIELD_OPTS,MP_MAP); + if (space_opts == NULL) + return -1; struct space_opts opts; struct region *region = &fiber()->gc; space_opts_decode(&opts, space_opts, region); struct sql_trigger *new_trigger = sql_trigger_compile(sql_get(), opts.sql); if (new_trigger == NULL) - diag_raise(); + return -1; auto new_trigger_guard = make_scoped_guard([=] { sql_trigger_delete(sql_get(), new_trigger); @@ -4112,24 +4295,27 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) if (strlen(trigger_name) != trigger_name_len || memcmp(trigger_name_src, trigger_name, trigger_name_len) != 0) { - tnt_raise(ClientError, ER_SQL_EXECUTE, + diag_set(ClientError, ER_SQL_EXECUTE, "trigger name does not match extracted " "from SQL"); + return -1; } - uint32_t space_id = - tuple_field_u32_xc(new_tuple, - BOX_TRIGGER_FIELD_SPACE_ID); + uint32_t space_id; + if (tuple_field_u32(new_tuple,BOX_TRIGGER_FIELD_SPACE_ID, + &space_id) != 0) + return -1; if (space_id != sql_trigger_space_id(new_trigger)) { - tnt_raise(ClientError, ER_SQL_EXECUTE, + diag_set(ClientError, ER_SQL_EXECUTE, "trigger space_id does not match the value " "resolved on AST building from SQL"); + return -1; } struct sql_trigger *old_trigger; if (sql_trigger_replace(trigger_name, sql_trigger_space_id(new_trigger), new_trigger, &old_trigger) != 0) - diag_raise(); + return -1; on_commit->data = old_trigger; if (old_tuple != NULL) { @@ -4448,20 +4634,24 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) struct fk_constraint_def *fk_def = fk_constraint_def_new_from_tuple(new_tuple, ER_CREATE_FK_CONSTRAINT); + if (fk_def == NULL) + return -1; auto fk_def_guard = make_scoped_guard([=] { free(fk_def); }); struct space *child_space = space_cache_find_xc(fk_def->child_id); if (child_space->def->opts.is_view) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "referencing space can't be VIEW"); + return -1; } struct space *parent_space = space_cache_find_xc(fk_def->parent_id); if (parent_space->def->opts.is_view) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "referenced space can't be VIEW"); + return -1; } /* * FIXME: until SQL triggers are completely @@ -4472,9 +4662,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) */ struct index *pk = space_index(child_space, 0); if (pk != NULL && index_size(pk) > 0) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "referencing space must be empty"); + return -1; } /* Check types of referenced fields. */ for (uint32_t i = 0; i < fk_def->field_count; ++i) { @@ -4482,9 +4673,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) uint32_t parent_fieldno = fk_def->links[i].parent_field; if (child_fieldno >= child_space->def->field_count || parent_fieldno >= parent_space->def->field_count) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "foreign key refers to " - "nonexistent field"); + "nonexistent field"); + return -1; } struct field_def *child_field = &child_space->def->fields[child_fieldno]; @@ -4492,13 +4684,15 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) &parent_space->def->fields[parent_fieldno]; if (! field_type1_contains_type2(parent_field->type, child_field->type)) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "field type mismatch"); + return -1; } if (child_field->coll_id != parent_field->coll_id) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "field collation mismatch"); + return -1; } } fk_constraint_check_dup_links(fk_def); @@ -4520,8 +4714,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) for (j = 0; j < fk_def->field_count; ++j) { if (key_def_find_by_fieldno(idx->def->key_def, fk_def->links[j]. - parent_field) == - NULL) + parent_field) == + NULL) break; } if (j != fk_def->field_count) @@ -4530,15 +4724,17 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) break; } if (fk_index == NULL) { - tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, "referenced fields don't " "compose unique index"); + return -1; } struct fk_constraint *fk = (struct fk_constraint *) malloc(sizeof(*fk)); if (fk == NULL) { - tnt_raise(OutOfMemory, sizeof(*fk), + diag_set(OutOfMemory, sizeof(*fk), "malloc", "struct fk_constraint"); + return -1; } auto fk_guard = make_scoped_guard([=] { free(fk); }); memset(fk, 0, sizeof(*fk)); @@ -4552,6 +4748,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) struct trigger *on_rollback = txn_alter_trigger_new(on_create_fk_constraint_rollback, fk); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); fk_constraint_set_mask(fk, &parent_space->fk_constraint_mask, @@ -4570,10 +4768,14 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) struct trigger *on_rollback = txn_alter_trigger_new(on_replace_fk_constraint_rollback, old_fk); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); struct trigger *on_commit = txn_alter_trigger_new(on_drop_or_replace_fk_constraint_commit, old_fk); + if (on_commit == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit); space_reset_fk_constraint_mask(child_space); space_reset_fk_constraint_mask(parent_space); @@ -4596,10 +4798,14 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) struct trigger *on_commit = txn_alter_trigger_new(on_drop_or_replace_fk_constraint_commit, old_fk); + if (on_commit == NULL) + return -1; txn_stmt_on_commit(stmt, on_commit); struct trigger *on_rollback = txn_alter_trigger_new(on_drop_fk_constraint_rollback, old_fk); + if (on_rollback == NULL) + return -1; txn_stmt_on_rollback(stmt, on_rollback); space_reset_fk_constraint_mask(child_space); space_reset_fk_constraint_mask(parent_space); @@ -4727,14 +4933,18 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) struct space *space = space_cache_find_xc(space_id); struct trigger *on_rollback = txn_alter_trigger_new(NULL, NULL); struct trigger *on_commit = txn_alter_trigger_new(NULL, NULL); + if (on_commit == NULL || on_rollback == NULL) + return -1; if (new_tuple != NULL) { - bool is_deferred = - tuple_field_bool_xc(new_tuple, - BOX_CK_CONSTRAINT_FIELD_DEFERRED); + bool is_deferred; + if (tuple_field_bool(new_tuple, + BOX_CK_CONSTRAINT_FIELD_DEFERRED, &is_deferred) != 0) + return -1; if (is_deferred) { - tnt_raise(ClientError, ER_UNSUPPORTED, "Tarantool", + diag_set(ClientError, ER_UNSUPPORTED, "Tarantool", "deferred ck constraints"); + return -1; } /* Create or replace check constraint. */ struct ck_constraint_def *ck_def = @@ -4748,14 +4958,15 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) */ struct index *pk = space_index(space, 0); if (pk != NULL && index_size(pk) > 0) { - tnt_raise(ClientError, ER_CREATE_CK_CONSTRAINT, + diag_set(ClientError, ER_CREATE_CK_CONSTRAINT, ck_def->name, "referencing space must be empty"); + return -1; } struct ck_constraint *new_ck_constraint = ck_constraint_new(ck_def, space->def); if (new_ck_constraint == NULL) - diag_raise(); + return -1; ck_def_guard.is_active = false; auto ck_guard = make_scoped_guard([=] { ck_constraint_delete(new_ck_constraint); @@ -4765,8 +4976,9 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) space_ck_constraint_by_name(space, name, strlen(name)); if (old_ck_constraint != NULL) rlist_del_entry(old_ck_constraint, link); - if (space_add_ck_constraint(space, new_ck_constraint) != 0) - diag_raise(); + if (space_add_ck_constraint(space, new_ck_constraint) != 0) { + return -1; + } ck_guard.is_active = false; if (old_tuple != NULL) { on_rollback->data = old_ck_constraint; @@ -4781,10 +4993,10 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) assert(new_tuple == NULL && old_tuple != NULL); /* Drop check constraint. */ uint32_t name_len; - const char *name = - tuple_field_str_xc(old_tuple, - BOX_CK_CONSTRAINT_FIELD_NAME, - &name_len); + const char *name = tuple_field_str(old_tuple, + BOX_CK_CONSTRAINT_FIELD_NAME, &name_len); + if (name == NULL) + return -1; struct ck_constraint *old_ck_constraint = space_ck_constraint_by_name(space, name, name_len); assert(old_ck_constraint != NULL); @@ -4817,15 +5029,28 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) struct index *index; struct space *space; if (old_tuple == NULL && new_tuple != NULL) { - uint32_t space_id = tuple_field_u32_xc(new_tuple, - BOX_FUNC_INDEX_FIELD_SPACE_ID); - uint32_t index_id = tuple_field_u32_xc(new_tuple, - BOX_FUNC_INDEX_FIELD_INDEX_ID); - uint32_t fid = tuple_field_u32_xc(new_tuple, - BOX_FUNC_INDEX_FUNCTION_ID); - space = space_cache_find_xc(space_id); - index = index_find_xc(space, index_id); - func = func_cache_find(fid); + uint32_t space_id; + uint32_t index_id; + uint32_t fid; + if (tuple_field_u32(new_tuple,BOX_FUNC_INDEX_FIELD_SPACE_ID, + &space_id) != 0) + return -1; + if (tuple_field_u32(new_tuple,BOX_FUNC_INDEX_FIELD_INDEX_ID, + &index_id) != 0) + return -1; + if (tuple_field_u32(new_tuple,BOX_FUNC_INDEX_FUNCTION_ID, &fid) != 0) + return -1; + space = space_cache_find(space_id); + if (space == NULL) + return -1; + index = index_find(space, index_id); + if (index == NULL) + return -1; + func = func_by_id(fid); + if (func == NULL) { + diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(fid)); + return -1; + } func_index_check_func(func); if (index->def->opts.func_id != func->def->fid) { tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS, 0, @@ -4833,17 +5058,26 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) "_func_index don't match"); } } else if (old_tuple != NULL && new_tuple == NULL) { - uint32_t space_id = tuple_field_u32_xc(old_tuple, - BOX_FUNC_INDEX_FIELD_SPACE_ID); - uint32_t index_id = tuple_field_u32_xc(old_tuple, - BOX_FUNC_INDEX_FIELD_INDEX_ID); - space = space_cache_find_xc(space_id); - index = index_find_xc(space, index_id); + uint32_t space_id; + uint32_t index_id; + + if (tuple_field_u32(old_tuple,BOX_FUNC_INDEX_FIELD_SPACE_ID, + &space_id) != 0) + return -1; + if (tuple_field_u32(old_tuple,BOX_FUNC_INDEX_FIELD_INDEX_ID, + &index_id) != 0) + return -1; + space = space_cache_find(space_id); + if (space == NULL) + return -1; + index = index_find(space, index_id); + if (index == NULL) + return -1; func = NULL; } else { assert(old_tuple != NULL && new_tuple != NULL); - tnt_raise(ClientError, ER_UNSUPPORTED, - "functional index", "alter"); + diag_set(ClientError, ER_UNSUPPORTED, "functional index", "alter"); + return -1; } /** diff --git a/src/box/replication.cc b/src/box/replication.cc index 82bf496c8..edb036e96 100644 --- a/src/box/replication.cc +++ b/src/box/replication.cc @@ -114,15 +114,19 @@ replication_free(void) free(replicaset.replica_by_id); } -void +int replica_check_id(uint32_t replica_id) { - if (replica_id == REPLICA_ID_NIL) - tnt_raise(ClientError, ER_REPLICA_ID_IS_RESERVED, - (unsigned) replica_id); - if (replica_id >= VCLOCK_MAX) - tnt_raise(LoggedError, ER_REPLICA_MAX, - (unsigned) replica_id); + if (replica_id == REPLICA_ID_NIL) { + diag_set(ClientError, ER_REPLICA_ID_IS_RESERVED, + (unsigned) replica_id); + return -1; + } + if (replica_id >= VCLOCK_MAX) { + diag_set(ClientError, ER_REPLICA_MAX, + (unsigned) replica_id); + return -1; + } /* * It's okay to update the instance id while it is joining to * a cluster as long as the id is set by the time bootstrap is @@ -133,9 +137,12 @@ replica_check_id(uint32_t replica_id) * case it will replay this operation during the final join * stage. */ - if (!replicaset.is_joining && replica_id == instance_id) - tnt_raise(ClientError, ER_LOCAL_INSTANCE_ID_IS_READ_ONLY, - (unsigned) replica_id); + if (!replicaset.is_joining && replica_id == instance_id) { + diag_set(ClientError, ER_LOCAL_INSTANCE_ID_IS_READ_ONLY, + (unsigned) replica_id); + return -1; + } + return 0; } /* Return true if replica doesn't have id, relay and applier */ @@ -154,7 +161,7 @@ static struct replica * replica_new(void) { struct replica *replica = (struct replica *) - malloc(sizeof(struct replica)); + malloc(sizeof(struct replica)); if (replica == NULL) { tnt_raise(OutOfMemory, sizeof(*replica), "malloc", "struct replica"); @@ -567,24 +574,24 @@ replicaset_update(struct applier **appliers, int count) * Replica set configuration state, shared among appliers. */ struct replicaset_connect_state { - /** Number of successfully connected appliers. */ - int connected; - /** Number of appliers that failed to connect. */ - int failed; - /** Signaled when an applier connects or stops. */ - struct fiber_cond wakeup; + /** Number of successfully connected appliers. */ + int connected; + /** Number of appliers that failed to connect. */ + int failed; + /** Signaled when an applier connects or stops. */ + struct fiber_cond wakeup; }; struct applier_on_connect { - struct trigger base; - struct replicaset_connect_state *state; + struct trigger base; + struct replicaset_connect_state *state; }; static int applier_on_connect_f(struct trigger *trigger, void *event) { struct applier_on_connect *on_connect = container_of(trigger, - struct applier_on_connect, base); + struct applier_on_connect, base); struct replicaset_connect_state *state = on_connect->state; struct applier *applier = (struct applier *)event; @@ -705,7 +712,7 @@ replicaset_connect(struct applier **appliers, int count, goto error; } return; -error: + error: /* Destroy appliers */ for (int i = 0; i < count; i++) { trigger_clear(&triggers[i].base); @@ -736,7 +743,7 @@ replicaset_needs_rejoin(struct replica **master) const char *uuid_str = tt_uuid_str(&replica->uuid); const char *addr_str = sio_strfaddr(&applier->addr, - applier->addr_len); + applier->addr_len); const char *local_vclock_str = vclock_to_string(&replicaset.vclock); const char *remote_vclock_str = vclock_to_string(&ballot->vclock); const char *gc_vclock_str = vclock_to_string(&ballot->gc_vclock); @@ -905,7 +912,7 @@ replicaset_round(bool skip_ro) * the lowest uuid. */ int cmp = vclock_compare(&applier->ballot.vclock, - &leader->applier->ballot.vclock); + &leader->applier->ballot.vclock); if (cmp < 0) continue; if (cmp == 0 && tt_uuid_compare(&replica->uuid, diff --git a/src/box/replication.h b/src/box/replication.h index 19f283c7d..470420592 100644 --- a/src/box/replication.h +++ b/src/box/replication.h @@ -352,7 +352,7 @@ replica_on_relay_stop(struct replica *replica); #if defined(__cplusplus) } /* extern "C" */ -void +int replica_check_id(uint32_t replica_id); /** diff --git a/src/box/schema.cc b/src/box/schema.cc index 8d8aae448..9767207e0 100644 --- a/src/box/schema.cc +++ b/src/box/schema.cc @@ -599,12 +599,22 @@ func_by_name(const char *name, uint32_t name_len) return (struct func *) mh_strnptr_node(funcs_by_name, func)->val; } -bool -schema_find_grants(const char *type, uint32_t id) +int +schema_find_grants(const char *type, uint32_t id, bool *out) { - struct space *priv = space_cache_find_xc(BOX_PRIV_ID); + struct space *priv = space_cache_find(BOX_PRIV_ID); + if (priv == NULL) + return -1; + /** "object" index */ - struct index *index = index_find_system_xc(priv, 2); + if (!space_is_memtx(priv)) { + diag_set(ClientError, ER_UNSUPPORTED, + priv->engine->name, "system data"); + return -1; + } + struct index *index = index_find(priv, 2); + if (index == NULL) + return -1; /* * +10 = max(mp_sizeof_uint32) + * max(mp_sizeof_strl(uint32)). @@ -612,9 +622,15 @@ schema_find_grants(const char *type, uint32_t id) char key[GRANT_NAME_MAX + 10]; assert(strlen(type) <= GRANT_NAME_MAX); mp_encode_uint(mp_encode_str(key, type, strlen(type)), id); - struct iterator *it = index_create_iterator_xc(index, ITER_EQ, key, 2); + struct iterator *it = index_create_iterator(index, ITER_EQ, key, 2); + if (it == NULL) + return -1; IteratorGuard iter_guard(it); - return iterator_next_xc(it); + struct tuple *tuple; + if (iterator_next(it, &tuple) != 0) + return -1; + *out = (tuple != NULL); + return 0; } struct sequence * diff --git a/src/box/schema.h b/src/box/schema.h index f9d15b38d..88bfd74ad 100644 --- a/src/box/schema.h +++ b/src/box/schema.h @@ -171,15 +171,6 @@ schema_free(); struct space *schema_space(uint32_t id); -static inline struct func * -func_cache_find(uint32_t fid) -{ - struct func *func = func_by_id(fid); - if (func == NULL) - tnt_raise(ClientError, ER_NO_SUCH_FUNCTION, int2str(fid)); - return func; -} - /** * Check whether or not an object has grants on it (restrict @@ -188,8 +179,8 @@ func_cache_find(uint32_t fid) * @retval true object has grants * @retval false object has no grants */ -bool -schema_find_grants(const char *type, uint32_t id); +int +schema_find_grants(const char *type, uint32_t id, bool *out); /** * A wrapper around sequence_by_id() that raises an exception diff --git a/src/box/sequence.h b/src/box/sequence.h index 976020a25..a164da9af 100644 --- a/src/box/sequence.h +++ b/src/box/sequence.h @@ -179,15 +179,6 @@ sequence_get_value(struct sequence *seq); #if defined(__cplusplus) } /* extern "C" */ -static inline struct sequence * -sequence_new_xc(struct sequence_def *def) -{ - struct sequence *seq = sequence_new(def); - if (seq == NULL) - diag_raise(); - return seq; -} - #endif /* defined(__cplusplus) */ #endif /* INCLUDES_TARANTOOL_BOX_SEQUENCE_H */ diff --git a/src/box/user.cc b/src/box/user.cc index c46ff67d1..50614c6f2 100644 --- a/src/box/user.cc +++ b/src/box/user.cc @@ -339,7 +339,8 @@ user_reload_privs(struct user *user) struct tuple *tuple; while ((tuple = iterator_next_xc(it)) != NULL) { struct priv_def priv; - priv_def_create_from_tuple(&priv, tuple); + if (priv_def_create_from_tuple(&priv, tuple) != 0) + diag_raise(); /** * Skip role grants, we're only * interested in real objects. @@ -559,7 +560,7 @@ user_cache_free() /** {{{ roles */ -void +int role_check(struct user *grantee, struct user *role) { /* @@ -592,9 +593,11 @@ role_check(struct user *grantee, struct user *role) */ if (user_map_is_set(&transitive_closure, role->auth_token)) { - tnt_raise(ClientError, ER_ROLE_LOOP, + diag_set(ClientError, ER_ROLE_LOOP, role->def->name, grantee->def->name); + return -1; } + return 0; } /** diff --git a/src/box/user.h b/src/box/user.h index 527fb2e7c..526cd39ca 100644 --- a/src/box/user.h +++ b/src/box/user.h @@ -144,16 +144,6 @@ user_cache_replace(struct user_def *user); void user_cache_delete(uint32_t uid); -/* Find a user by name. Used by authentication. */ -static inline struct user * -user_find_xc(uint32_t uid) -{ - struct user *user = user_find(uid); - if (user == NULL) - diag_raise(); - return user; -} - static inline struct user * user_find_by_name_xc(const char *name, uint32_t len) { @@ -178,7 +168,7 @@ user_cache_free(); * and no loop in the graph will occur when grantee gets * a given role. */ -void +int role_check(struct user *grantee, struct user *role); /** @@ -201,7 +191,7 @@ role_revoke(struct user *grantee, struct user *role); void priv_grant(struct user *grantee, struct priv_def *priv); -void +int priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple); /* }}} */ -- 2.17.1