[tarantool-patches] Re: [PATCH v2 2/6] refactoring: remove exceptions from used in alter.cc outer functions

Georgy Kirichenko georgy at tarantool.org
Tue Aug 27 11:37:53 MSK 2019


There are some comments for this patch:
it's ok to write _xc inline function in header. At least it's better than:
if (<func>(...)) 
   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);
> 
>  /* }}} */








More information about the Tarantool-patches mailing list