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 3551A25528 for ; Tue, 27 Aug 2019 04:37:56 -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 0fMs1sHOjVMN for ; Tue, 27 Aug 2019 04:37:56 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 C83562551E for ; Tue, 27 Aug 2019 04:37:55 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] Re: [PATCH v2 2/6] refactoring: remove exceptions from used in alter.cc outer functions Date: Tue, 27 Aug 2019 11:37:53 +0300 Message-ID: <2504980.YmMpLnKspn@home.lan> In-Reply-To: <1738e5ff776683bf69165878b788291ff7eb1455.1565979897.git.i.kosarev@tarantool.org> References: <1738e5ff776683bf69165878b788291ff7eb1455.1565979897.git.i.kosarev@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: Ilya Kosarev Cc: tarantool-patches@freelists.org, i.kosarev@corp.mail.ru There are some comments for this patch: it's ok to write _xc inline function in header. At least it's better than: if ((...)) diag_raise() case idems should use the same indentation as switch ones don't change indentation from tabs to spaces (we dont use more than 7 spaces consequently) On Friday, August 16, 2019 9:37:48 PM MSK Ilya Kosarev wrote: > Outer functions, used in alter.cc, are cleared from triggers. > Their usage in alter.cc is updated. > --- > src/box/alter.cc | 169 +++++++++++++++++++++++++++-------------- > src/box/identifier.h | 10 --- > src/box/replication.cc | 115 +++++++++++++++------------- > 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 +--- > 9 files changed, 208 insertions(+), 161 deletions(-) > > diff --git a/src/box/alter.cc b/src/box/alter.cc > index 81edd62b5..6b51fdc16 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) { > @@ -285,7 +287,8 @@ index_def_new_from_tuple(struct tuple *tuple, struct > space *space) tt_cstr(name, BOX_INVALID_NAME_MAX), > space_name(space), "index name is too long"); > } > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > struct key_def *key_def = NULL; > struct key_part_def *part_def = (struct key_part_def *) > malloc(sizeof(*part_def) * part_count); > @@ -430,7 +433,8 @@ field_def_decode(struct field_def *field, const char > **data, tt_sprintf("field %d name is too long", > fieldno + TUPLE_INDEX_BASE)); > } > - identifier_check_xc(field->name, field_name_len); > + if (identifier_check(field->name, field_name_len) != 0) > + diag_raise(); > if (field->type == field_type_MAX) { > tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len), > tt_sprintf("field %d has unknown field type", > @@ -524,7 +528,8 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t > errcode, tnt_raise(ClientError, errcode, > tt_cstr(name, BOX_INVALID_NAME_MAX), > "space name is too long"); > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > uint32_t id = tuple_field_u32_xc(tuple, BOX_SPACE_FIELD_ID); > if (id > BOX_SPACE_MAX) { > tnt_raise(ClientError, errcode, tt_cstr(name, name_len), > @@ -549,7 +554,8 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t > errcode, tnt_raise(ClientError, errcode, tt_cstr(name, name_len), > "space engine name is too long"); > } > - identifier_check_xc(engine_name, engine_name_len); > + if (identifier_check(engine_name, engine_name_len) != 0) > + diag_raise(); > struct field_def *fields; > uint32_t field_count; > /* Check space opts. */ > @@ -1986,10 +1992,15 @@ on_replace_dd_space(struct trigger * /* trigger */, > void *event) space_name(old_space), > "the space has indexes"); > } > - 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, > @@ -2553,7 +2564,8 @@ user_def_new_from_tuple(struct tuple *tuple) > tnt_raise(ClientError, ER_CREATE_USER, > user->name, "unknown user type"); > } > - identifier_check_xc(user->name, name_len); > + if (identifier_check(user->name, name_len) != 0) > + diag_raise(); > /* > * AUTH_DATA field in _user space should contain > * chap-sha1 -> base64_encode(sha1(sha1(password), 0). > @@ -2698,7 +2710,8 @@ func_def_new_from_tuple(struct tuple *tuple) > tt_cstr(name, BOX_INVALID_NAME_MAX), > "function name is too long"); > } > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > if (field_count > BOX_FUNC_FIELD_BODY) { > body = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_BODY, > &body_len); > @@ -2933,10 +2946,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)) { > @@ -2985,7 +3003,8 @@ coll_id_def_new_from_tuple(struct tuple *tuple, struct > coll_id_def *def) if (name_len > BOX_NAME_MAX) > tnt_raise(ClientError, ER_CANT_CREATE_COLLATION, > "collation name is too long"); > - identifier_check_xc(def->name, name_len); > + if (identifier_check(def->name, name_len) != 0) > + diag_raise(); > > def->owner_id = tuple_field_u32_xc(tuple, BOX_COLLATION_FIELD_UID); > struct coll_def *base = &def->base; > @@ -3002,7 +3021,8 @@ coll_id_def_new_from_tuple(struct tuple *tuple, struct > coll_id_def *def) tnt_raise(ClientError, ER_CANT_CREATE_COLLATION, > "collation locale is too long"); > if (locale_len > 0) > - identifier_check_xc(locale, locale_len); > + if (identifier_check(locale, locale_len) != 0) > + diag_raise(); > snprintf(base->locale, sizeof(base->locale), "%.*s", locale_len, > locale); > const char *options = > @@ -3171,20 +3191,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, > - BOX_PRIV_FIELD_OBJECT_ID + TUPLE_INDEX_BASE); > + 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 > @@ -3193,23 +3218,28 @@ priv_def_create_from_tuple(struct priv_def *priv, > struct tuple *tuple) * So check for that first. > */ > switch (mp_typeof(*data)) { > - case MP_STR: > - if (mp_decode_strl(&data) == 0) { > - /* Entity-wide privilege. */ > - priv->object_id = 0; > - priv->object_type = schema_entity_type(priv- >object_type); > - break; > - } > - FALLTHROUGH; > - default: > - priv->object_id = tuple_field_u32_xc(tuple, > - BOX_PRIV_FIELD_OBJECT_ID); > + case MP_STR: > + if (mp_decode_strl(&data) == 0) { > + /* Entity-wide privilege. */ > + priv->object_id = 0; > + priv->object_type = schema_entity_type(priv- >object_type); > + break; > + } > + FALLTHROUGH; > + default: > + 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, > - object_type); > + 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; > } > > /* > @@ -3225,7 +3255,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) { > @@ -3259,7 +3291,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, > @@ -3302,7 +3338,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: > @@ -3378,7 +3415,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; > @@ -3391,7 +3429,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; > } > @@ -3410,7 +3449,8 @@ 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 = > @@ -3418,7 +3458,8 @@ on_replace_dd_priv(struct trigger * /* trigger */, > void *event) 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); > @@ -3426,7 +3467,8 @@ on_replace_dd_priv(struct trigger * /* trigger */, > void *event) txn_alter_trigger_new(modify_priv, old_tuple); > 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 = > @@ -3540,7 +3582,8 @@ on_replace_dd_cluster(struct trigger *trigger, void > *event) /* Check fields */ > uint32_t replica_id = > tuple_field_u32_xc(new_tuple, BOX_CLUSTER_FIELD_ID); > - replica_check_id(replica_id); > + if (replica_check_id(replica_id) != 0) > + return -1; > tt_uuid replica_uuid; > tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, > &replica_uuid); > @@ -3575,7 +3618,8 @@ on_replace_dd_cluster(struct trigger *trigger, void > *event) assert(old_tuple != NULL); > uint32_t replica_id = > tuple_field_u32_xc(old_tuple, BOX_CLUSTER_FIELD_ID); > - replica_check_id(replica_id); > + if (replica_check_id(replica_id) != 0) > + return -1; > > struct trigger *on_commit; > on_commit = txn_alter_trigger_new(unregister_replica, > @@ -3601,7 +3645,8 @@ sequence_def_new_from_tuple(struct tuple *tuple, > uint32_t errcode) tt_cstr(name, BOX_INVALID_NAME_MAX), > "sequence name is too long"); > } > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > size_t sz = sequence_def_sizeof(name_len); > struct sequence_def *def = (struct sequence_def *) malloc(sz); > if (def == NULL) > @@ -3706,7 +3751,9 @@ 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); > + 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); > @@ -3717,15 +3764,21 @@ on_replace_dd_sequence(struct trigger * /* trigger > */, void *event) 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, > - seq->def->name, "the sequence has grants"); > + 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 = > @@ -4210,7 +4263,8 @@ fk_constraint_def_new_from_tuple(struct tuple *tuple, > uint32_t errcode) tt_cstr(name, BOX_INVALID_NAME_MAX), > "constraint name is too long"); > } > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > uint32_t link_count; > struct field_link *links = decode_fk_links(tuple, &link_count, name, > name_len, errcode); > @@ -4614,7 +4668,8 @@ ck_constraint_def_new_from_tuple(struct tuple *tuple) > tt_cstr(name, BOX_INVALID_NAME_MAX), > "check constraint name is too long"); > } > - identifier_check_xc(name, name_len); > + if (identifier_check(name, name_len) != 0) > + diag_raise(); > uint32_t space_id = > tuple_field_u32_xc(tuple, BOX_CK_CONSTRAINT_FIELD_SPACE_ID); > const char *language_str = > @@ -4819,7 +4874,11 @@ on_replace_dd_func_index(struct trigger *trigger, > void *event) BOX_FUNC_INDEX_FUNCTION_ID); > space = space_cache_find_xc(space_id); > index = index_find_xc(space, index_id); > - func = func_cache_find(fid); > + 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, > diff --git a/src/box/identifier.h b/src/box/identifier.h > index a0ed6c10e..0d39793ba 100644 > --- a/src/box/identifier.h > +++ b/src/box/identifier.h > @@ -51,16 +51,6 @@ identifier_check(const char *str, int str_len); > #if defined(__cplusplus) > } /* extern "C" */ > > -/** > - * Throw an error if identifier is not valid. > - */ > -static inline void > -identifier_check_xc(const char *str, int str_len) > -{ > - if (identifier_check(str, str_len)) > - diag_raise(); > -} > - > #endif /* defined(__cplusplus) */ > > #endif /* TARANTOOL_BOX_IDENTIFIER_H_INCLUDED */ > diff --git a/src/box/replication.cc b/src/box/replication.cc > index 41b83a3ad..e8a1890e7 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"); > @@ -379,22 +386,22 @@ static void > replica_on_applier_disconnect(struct replica *replica) > { > switch (replica->applier_sync_state) { > - case APPLIER_SYNC: > - assert(replicaset.applier.synced > 0); > - replicaset.applier.synced--; > - FALLTHROUGH; > - case APPLIER_CONNECTED: > - assert(replicaset.applier.connected > 0); > - replicaset.applier.connected--; > - break; > - case APPLIER_LOADING: > - assert(replicaset.applier.loading > 0); > - replicaset.applier.loading--; > - break; > - case APPLIER_DISCONNECTED: > - break; > - default: > - unreachable(); > + case APPLIER_SYNC: > + assert(replicaset.applier.synced > 0); > + replicaset.applier.synced--; > + FALLTHROUGH; > + case APPLIER_CONNECTED: > + assert(replicaset.applier.connected > 0); > + replicaset.applier.connected--; > + break; > + case APPLIER_LOADING: > + assert(replicaset.applier.loading > 0); > + replicaset.applier.loading--; > + break; > + case APPLIER_DISCONNECTED: > + break; > + default: > + unreachable(); > } > replica->applier_sync_state = replica->applier->state; > if (replica->applier_sync_state == APPLIER_LOADING) > @@ -406,7 +413,7 @@ replica_on_applier_state_f(struct trigger *trigger, void > *event) { > (void)event; > struct replica *replica = container_of(trigger, > - struct replica, on_applier_state); > + struct replica, on_applier_state); > switch (replica->applier->state) { > case APPLIER_INITIAL_JOIN: > replicaset.is_joining = true; > @@ -465,11 +472,11 @@ replicaset_update(struct applier **appliers, int > count) struct applier *applier; > > auto uniq_guard = make_scoped_guard([&]{ > - replica_hash_foreach_safe(&uniq, replica, next) { > - replica_hash_remove(&uniq, replica); > - replica_clear_applier(replica); > - replica_delete(replica); > - } > + replica_hash_foreach_safe(&uniq, replica, next) { > + replica_hash_remove(&uniq, replica); > + replica_clear_applier(replica); > + replica_delete(replica); > + } > }); > > /* Check for duplicate UUID */ > @@ -567,37 +574,37 @@ 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; > > switch (applier->state) { > - case APPLIER_OFF: > - case APPLIER_STOPPED: > - state->failed++; > - break; > - case APPLIER_CONNECTED: > - state->connected++; > - break; > - default: > - return 0; > + case APPLIER_OFF: > + case APPLIER_STOPPED: > + state->failed++; > + break; > + case APPLIER_CONNECTED: > + state->connected++; > + break; > + default: > + return 0; > } > fiber_cond_signal(&state->wakeup); > applier_pause(applier); > @@ -694,7 +701,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); > @@ -725,7 +732,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); > @@ -894,7 +901,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); > > /* }}} */