<HTML><BODY><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">In v5 I will fix this.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><br><span style="color: #222222; font-family: Rubik, Arial, sans-serif; font-size: 17px;" data-mce-style="color: #222222; font-family: Rubik, Arial, sans-serif; font-size: 17px;">Sincerely,<br></span>Ilya Kosarev</p><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Понедельник, 30 сентября 2019, 15:28 +03:00 от Georgy Kirichenko <georgy@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15698465260108708980_BODY">Hi, the patch is mostly Ok, please see some comments below<br>
<br>
On Monday, September 23, 2019 6:56:56 PM MSK Ilya Kosarev wrote:<br>
> /**<br>
> @@ -708,8 +710,15 @@ public:<br>
> static struct trigger *<br>
> txn_alter_trigger_new(trigger_f run, void *data)<br>
> {<br>
> + size_t size = sizeof(struct trigger);<br>
> struct trigger *trigger = (struct trigger *)<br>
> - region_calloc_object_xc(&in_txn()->region, struct <br>
trigger);<br>
> + region_aligned_alloc(&in_txn()->region, size,<br>
> + alignof(struct trigger));<br>
> + if (trigger == NULL) {<br>
> + diag_set(OutOfMemory, size, "region", "new slab");<br>
Please use a structure name instead of a 'new slab' abbreviation (you can see <br>
an example after OutOfMemory search)<br>
> + return NULL;<br>
> + }<br>
> + trigger = (struct trigger *)memset(trigger, 0, size);<br>
> trigger->run = run;<br>
> trigger->data = data;<br>
> trigger->destroy = NULL;<br>
> @@ -979,6 +988,8 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space<br>
> *alter) struct trigger *on_commit, *on_rollback;<br>
> on_commit = txn_alter_trigger_new(alter_space_commit, alter);<br>
> on_rollback = txn_alter_trigger_new(alter_space_rollback, alter);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + diag_raise();<br>
> <br>
> /* Create a definition of the new space. */<br>
> space_dump_def(alter->old_space, &alter->key_list);<br>
> @@ -1938,8 +1949,9 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) region);<br>
> auto def_guard =<br>
> make_scoped_guard([=] { <br>
space_def_delete(def); });<br>
> - access_check_ddl(def->name, def->id, def->uid, <br>
SC_SPACE,<br>
> - PRIV_C);<br>
> + if (access_check_ddl(def->name, def->id, def->uid, <br>
SC_SPACE,<br>
> + PRIV_C) != 0)<br>
> + return -1;<br>
> RLIST_HEAD(empty_list);<br>
> struct space *space = space_new_xc(def, &empty_list);<br>
> /**<br>
> @@ -1965,6 +1977,8 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) */<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_create_space_rollback, space);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> if (def->opts.is_view) {<br>
> struct Select *select = <br>
sql_view_compile(sql_get(),<br>
> @@ -1990,16 +2004,21 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) struct trigger *on_commit_view =<br>
> <br>
txn_alter_trigger_new(on_create_view_commit,<br>
> <br>
select);<br>
> + if (on_commit_view == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit_view);<br>
> struct trigger *on_rollback_view =<br>
> <br>
txn_alter_trigger_new(on_create_view_rollback,<br>
> <br>
select);<br>
> + if (on_rollback_view == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, <br>
on_rollback_view);<br>
> select_guard.is_active = false;<br>
> }<br>
> } else if (new_tuple == NULL) { /* DELETE */<br>
> - access_check_ddl(old_space->def->name, old_space->def-<br>
>id,<br>
> - old_space->def->uid, SC_SPACE, <br>
PRIV_D);<br>
> + if (access_check_ddl(old_space->def->name, old_space-<br>
>def->id,<br>
> + old_space->def->uid, SC_SPACE, <br>
PRIV_D) != 0)<br>
> + return -1;<br>
> /* Verify that the space is empty (has no indexes) */<br>
> if (old_space->index_count) {<br>
> diag_set(ClientError, ER_DROP_SPACE,<br>
> @@ -2058,9 +2077,13 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) ++schema_version;<br>
> struct trigger *on_commit =<br>
> txn_alter_trigger_new(on_drop_space_commit, <br>
old_space);<br>
> + if (on_commit == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_drop_space_rollback, old_space);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> if (old_space->def->opts.is_view) {<br>
> struct Select *select =<br>
> @@ -2074,10 +2097,14 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) struct trigger *on_commit_view =<br>
> <br>
txn_alter_trigger_new(on_drop_view_commit,<br>
> <br>
select);<br>
> + if (on_commit_view == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit_view);<br>
> struct trigger *on_rollback_view =<br>
> <br>
txn_alter_trigger_new(on_drop_view_rollback,<br>
> <br>
select);<br>
> + if (on_rollback_view == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, <br>
on_rollback_view);<br>
> update_view_references(select, -1, true, <br>
NULL);<br>
> select_guard.is_active = false;<br>
> @@ -2089,8 +2116,9 @@ on_replace_dd_space(struct trigger * /* trigger */,<br>
> void *event) region);<br>
> auto def_guard =<br>
> make_scoped_guard([=] { <br>
space_def_delete(def); });<br>
> - access_check_ddl(def->name, def->id, def->uid, <br>
SC_SPACE,<br>
> - PRIV_A);<br>
> + if (access_check_ddl(def->name, def->id, def->uid, <br>
SC_SPACE,<br>
> + PRIV_A) != 0)<br>
> + return -1;<br>
> if (def->id != space_id(old_space)) {<br>
> diag_set(ClientError, ER_ALTER_SPACE,<br>
> space_name(old_space),<br>
> @@ -2246,8 +2274,9 @@ on_replace_dd_index(struct trigger * /* trigger */,<br>
> void *event) enum priv_type priv_type = new_tuple ? PRIV_C : PRIV_D;<br>
> if (old_tuple && new_tuple)<br>
> priv_type = PRIV_A;<br>
> - access_check_ddl(old_space->def->name, old_space->def->id,<br>
> - old_space->def->uid, SC_SPACE, priv_type);<br>
> + if (access_check_ddl(old_space->def->name, old_space->def->id,<br>
> + old_space->def->uid, SC_SPACE, priv_type) != <br>
0)<br>
> + return -1;<br>
> struct index *old_index = space_index(old_space, iid);<br>
> <br>
> /*<br>
> @@ -2684,8 +2713,9 @@ on_replace_dd_user(struct trigger * /* trigger */,<br>
> void *event) struct user *old_user = user_by_id(uid);<br>
> if (new_tuple != NULL && old_user == NULL) { /* INSERT */<br>
> struct user_def *user = <br>
user_def_new_from_tuple(new_tuple);<br>
> - access_check_ddl(user->name, user->uid, user->owner, <br>
user->type,<br>
> - PRIV_C);<br>
> + if (access_check_ddl(user->name, user->uid, user->owner, <br>
user->type,<br>
> + PRIV_C) != 0)<br>
> + return -1;<br>
> auto def_guard = make_scoped_guard([=] { free(user); <br>
});<br>
> try {<br>
> (void) user_cache_replace(user);<br>
> @@ -2695,11 +2725,14 @@ on_replace_dd_user(struct trigger * /* trigger */,<br>
> void *event) def_guard.is_active = false;<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(user_cache_remove_user, new_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> } else if (new_tuple == NULL) { /* DELETE */<br>
> - access_check_ddl(old_user->def->name, old_user->def-<br>
>uid,<br>
> + if (access_check_ddl(old_user->def->name, old_user->def-<br>
>uid,<br>
> old_user->def->owner, old_user-<br>
>def->type,<br>
> - PRIV_D);<br>
> + PRIV_D) != 0)<br>
> + return -1;<br>
> /* Can't drop guest or super user */<br>
> if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == <br>
SUPER) {<br>
> diag_set(ClientError, ER_DROP_USER,<br>
> @@ -2719,6 +2752,8 @@ on_replace_dd_user(struct trigger * /* trigger */,<br>
> void *event) user_cache_delete(uid);<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(user_cache_alter_user, <br>
old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> } else { /* UPDATE, REPLACE */<br>
> assert(old_user != NULL && new_tuple != NULL);<br>
> @@ -2728,8 +2763,9 @@ on_replace_dd_user(struct trigger * /* trigger */,<br>
> void *event) * correct.<br>
> */<br>
> struct user_def *user = <br>
user_def_new_from_tuple(new_tuple);<br>
> - access_check_ddl(user->name, user->uid, user->uid,<br>
> - old_user->def->type, PRIV_A);<br>
> + if (access_check_ddl(user->name, user->uid, user->uid,<br>
> + old_user->def->type, PRIV_A) != <br>
0)<br>
> + return -1;<br>
> auto def_guard = make_scoped_guard([=] { free(user); <br>
});<br>
> try {<br>
> user_cache_replace(user);<br>
> @@ -2739,6 +2775,8 @@ on_replace_dd_user(struct trigger * /* trigger */,<br>
> void *event) def_guard.is_active = false;<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(user_cache_alter_user, <br>
old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> }<br>
> return 0;<br>
> @@ -2984,10 +3022,13 @@ on_replace_dd_func(struct trigger * /* trigger */,<br>
> void *event) if (new_tuple != NULL && old_func == NULL) { /* INSERT */<br>
> struct func_def *def = <br>
func_def_new_from_tuple(new_tuple);<br>
> auto def_guard = make_scoped_guard([=] { free(def); });<br>
> - access_check_ddl(def->name, def->fid, def->uid, <br>
SC_FUNCTION,<br>
> - PRIV_C);<br>
> + if (access_check_ddl(def->name, def->fid, def->uid, <br>
SC_FUNCTION,<br>
> + PRIV_C) != 0)<br>
> + return -1;<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_create_func_rollback, NULL);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> struct func *func = func_new(def);<br>
> if (func == NULL)<br>
> return -1;<br>
> @@ -3003,8 +3044,9 @@ on_replace_dd_func(struct trigger * /* trigger */,<br>
> void *event) * Can only delete func if you're the one<br>
> * who created it or a superuser.<br>
> */<br>
> - access_check_ddl(old_func->def->name, fid, uid, <br>
SC_FUNCTION,<br>
> - PRIV_D);<br>
> + if (access_check_ddl(old_func->def->name, fid, uid, <br>
SC_FUNCTION,<br>
> + PRIV_D) != 0)<br>
> + return -1;<br>
> /* Can only delete func if it has no grants. */<br>
> if (schema_find_grants("function", old_func->def->fid)) {<br>
> diag_set(ClientError, ER_DROP_FUNCTION,<br>
> @@ -3030,6 +3072,8 @@ on_replace_dd_func(struct trigger * /* trigger */,<br>
> void *event) txn_alter_trigger_new(on_drop_func_commit, old_func);<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(on_drop_func_rollback, <br>
old_func);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> func_cache_delete(old_func->def->fid);<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> @@ -3193,6 +3237,8 @@ on_replace_dd_collation(struct trigger * /* trigger<br>
> */, void *event) txn_alter_trigger_new(on_drop_collation_commit, NULL);<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_drop_collation_rollback, NULL);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> /*<br>
> * TODO: Check that no index uses the collation<br>
> * identifier.<br>
> @@ -3212,9 +3258,10 @@ on_replace_dd_collation(struct trigger * /* trigger<br>
> */, void *event) }<br>
> struct coll_id *old_coll_id = coll_by_id(old_id);<br>
> assert(old_coll_id != NULL);<br>
> - access_check_ddl(old_coll_id->name, old_coll_id->id,<br>
> + if (access_check_ddl(old_coll_id->name, old_coll_id->id,<br>
> old_coll_id->owner_id, <br>
SC_COLLATION,<br>
> - PRIV_D);<br>
> + PRIV_D) != 0)<br>
> + return -1;<br>
> /*<br>
> * Set on_commit/on_rollback triggers after<br>
> * deletion from the cache to make trigger logic<br>
> @@ -3229,10 +3276,13 @@ on_replace_dd_collation(struct trigger * /* trigger<br>
> */, void *event) /* INSERT */<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_create_collation_rollback, NULL);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> struct coll_id_def new_def;<br>
> coll_id_def_new_from_tuple(new_tuple, &new_def);<br>
> - access_check_ddl(new_def.name, new_def.id, <br>
new_def.owner_id,<br>
> - SC_COLLATION, PRIV_C);<br>
> + if (access_check_ddl(new_def.name, new_def.id, <br>
new_def.owner_id,<br>
> + SC_COLLATION, PRIV_C) != 0)<br>
> + return -1;<br>
> struct coll_id *new_coll_id = coll_id_new(&new_def);<br>
> if (new_coll_id == NULL)<br>
> return -1;<br>
> @@ -3256,20 +3306,25 @@ on_replace_dd_collation(struct trigger * /* trigger<br>
> */, void *event) /**<br>
> * Create a privilege definition from tuple.<br>
> */<br>
> -void<br>
> +int<br>
> priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)<br>
> {<br>
> - priv->grantor_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ID);<br>
> - priv->grantee_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_UID);<br>
> + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ID, &(priv->grantor_id)) !<br>
= 0)<br>
> + return -1;<br>
> + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &(priv->grantee_id)) <br>
!= 0)<br>
> + return -1;<br>
> <br>
> const char *object_type =<br>
> - tuple_field_cstr_xc(tuple, BOX_PRIV_FIELD_OBJECT_TYPE);<br>
> + tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE);<br>
> + if (object_type == NULL)<br>
> + return -1;<br>
> priv->object_type = schema_object_type(object_type);<br>
> <br>
> const char *data = tuple_field(tuple, BOX_PRIV_FIELD_OBJECT_ID);<br>
> if (data == NULL) {<br>
> - tnt_raise(ClientError, ER_NO_SUCH_FIELD_NO,<br>
> + diag_set(ClientError, ER_NO_SUCH_FIELD_NO,<br>
> BOX_PRIV_FIELD_OBJECT_ID + <br>
TUPLE_INDEX_BASE);<br>
> + return -1;<br>
> }<br>
> /*<br>
> * When granting or revoking privileges on a whole entity<br>
> @@ -3287,14 +3342,20 @@ priv_def_create_from_tuple(struct priv_def *priv,<br>
> struct tuple *tuple) }<br>
> FALLTHROUGH;<br>
> default:<br>
> - priv->object_id = tuple_field_u32_xc(tuple,<br>
> - <br>
BOX_PRIV_FIELD_OBJECT_ID);<br>
> + if (tuple_field_u32(tuple,<br>
> + BOX_PRIV_FIELD_OBJECT_ID, &(priv->object_id)) != 0)<br>
> + return -1;<br>
> }<br>
> if (priv->object_type == SC_UNKNOWN) {<br>
> - tnt_raise(ClientError, ER_UNKNOWN_SCHEMA_OBJECT,<br>
> + diag_set(ClientError, ER_UNKNOWN_SCHEMA_OBJECT,<br>
> object_type);<br>
> + return -1;<br>
> }<br>
> - priv->access = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ACCESS);<br>
> + uint32_t out;<br>
> + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ACCESS, &out) != 0)<br>
> + return -1;<br>
> + priv->access = out;<br>
> + return 0;<br>
> }<br>
> <br>
> /*<br>
> @@ -3307,62 +3368,80 @@ priv_def_create_from_tuple(struct priv_def *priv,<br>
> struct tuple *tuple) * object can be changed during WAL write.<br>
> * In the future we must protect grant/revoke with a logical lock.<br>
> */<br>
> -static void<br>
> +static int<br>
> priv_def_check(struct priv_def *priv, enum priv_type priv_type)<br>
> {<br>
> - struct user *grantor = user_find_xc(priv->grantor_id);<br>
> + struct user *grantor = user_find(priv->grantor_id);<br>
> + if (grantor == NULL)<br>
> + return -1;<br>
> /* May be a role */<br>
> struct user *grantee = user_by_id(priv->grantee_id);<br>
> if (grantee == NULL) {<br>
> - tnt_raise(ClientError, ER_NO_SUCH_USER,<br>
> + diag_set(ClientError, ER_NO_SUCH_USER,<br>
> int2str(priv->grantee_id));<br>
> + return -1;<br>
> }<br>
> const char *name = schema_find_name(priv->object_type, priv-<br>
>object_id);<br>
> - access_check_ddl(name, priv->object_id, grantor->def->uid,<br>
> - priv->object_type, priv_type);<br>
> + if (access_check_ddl(name, priv->object_id, grantor->def->uid,<br>
> + priv->object_type, priv_type) != 0)<br>
> + return -1;<br>
> switch (priv->object_type) {<br>
> case SC_UNIVERSE:<br>
> if (grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> <br>
schema_object_name(SC_UNIVERSE),<br>
> name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> break;<br>
> case SC_SPACE:<br>
> {<br>
> - struct space *space = space_cache_find_xc(priv-<br>
>object_id);<br>
> + struct space *space = space_cache_find(priv->object_id);<br>
> + if (space == NULL)<br>
> + return -1;<br>
> if (space->def->uid != grantor->def->uid &&<br>
> grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> schema_object_name(SC_SPACE), <br>
name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> break;<br>
> }<br>
> case SC_FUNCTION:<br>
> {<br>
> - struct func *func = func_cache_find(priv->object_id);<br>
> + struct func *func = func_by_id(priv->object_id);<br>
> + if (func == NULL) {<br>
> + diag_set(ClientError, ER_NO_SUCH_FUNCTION, <br>
int2str(priv->object_id));<br>
> + return -1;<br>
> + }<br>
> if (func->def->uid != grantor->def->uid &&<br>
> grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> <br>
schema_object_name(SC_FUNCTION), name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> break;<br>
> }<br>
> case SC_SEQUENCE:<br>
> {<br>
> - struct sequence *seq = sequence_cache_find(priv-<br>
>object_id);<br>
> + struct sequence *seq = sequence_by_id(priv->object_id);<br>
> + if (seq == NULL) {<br>
> + diag_set(ClientError, ER_NO_SUCH_SEQUENCE, <br>
int2str(priv->object_id));<br>
> + return -1;<br>
> + }<br>
> if (seq->def->uid != grantor->def->uid &&<br>
> grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> <br>
schema_object_name(SC_SEQUENCE), name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> break;<br>
> }<br>
> @@ -3370,9 +3449,10 @@ priv_def_check(struct priv_def *priv, enum priv_type<br>
> priv_type) {<br>
> struct user *role = user_by_id(priv->object_id);<br>
> if (role == NULL || role->def->type != SC_ROLE) {<br>
> - tnt_raise(ClientError, ER_NO_SUCH_ROLE,<br>
> + diag_set(ClientError, ER_NO_SUCH_ROLE,<br>
> role ? role->def->name :<br>
> int2str(priv->object_id));<br>
> + return -1;<br>
> }<br>
> /*<br>
> * Only the creator of the role can grant or revoke it.<br>
> @@ -3381,29 +3461,33 @@ priv_def_check(struct priv_def *priv, enum priv_type<br>
> priv_type) if (role->def->owner != grantor->def->uid &&<br>
> grantor->def->uid != ADMIN &&<br>
> (role->def->uid != PUBLIC || priv->access != <br>
PRIV_X)) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> schema_object_name(SC_ROLE), <br>
name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> /* Not necessary to do during revoke, but who cares. */<br>
> - role_check(grantee, role);<br>
> + if (role_check(grantee, role) != 0)<br>
> + return -1;<br>
> break;<br>
> }<br>
> case SC_USER:<br>
> {<br>
> struct user *user = user_by_id(priv->object_id);<br>
> if (user == NULL || user->def->type != SC_USER) {<br>
> - tnt_raise(ClientError, ER_NO_SUCH_USER,<br>
> + diag_set(ClientError, ER_NO_SUCH_USER,<br>
> user ? user->def->name :<br>
> int2str(priv->object_id));<br>
> + return -1;<br>
> }<br>
> if (user->def->owner != grantor->def->uid &&<br>
> grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError,<br>
> + diag_set(AccessDeniedError,<br>
> priv_name(priv_type),<br>
> schema_object_name(SC_USER), <br>
name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> break;<br>
> }<br>
> @@ -3415,30 +3499,33 @@ priv_def_check(struct priv_def *priv, enum priv_type<br>
> priv_type) {<br>
> /* Only admin may grant privileges on an entire entity. <br>
*/<br>
> if (grantor->def->uid != ADMIN) {<br>
> - tnt_raise(AccessDeniedError, <br>
priv_name(priv_type),<br>
> + diag_set(AccessDeniedError, <br>
priv_name(priv_type),<br>
> schema_object_name(priv-<br>
>object_type), name,<br>
> grantor->def->name);<br>
> + return -1;<br>
> }<br>
> }<br>
> default:<br>
> break;<br>
> }<br>
> if (priv->access == 0) {<br>
> - tnt_raise(ClientError, ER_GRANT,<br>
> + diag_set(ClientError, ER_GRANT,<br>
> "the grant tuple has no privileges");<br>
> + return -1;<br>
> }<br>
> + return 0;<br>
> }<br>
> <br>
> /**<br>
> * Update a metadata cache object with the new access<br>
> * data.<br>
> */<br>
> -static void<br>
> +static int<br>
> grant_or_revoke(struct priv_def *priv)<br>
> {<br>
> struct user *grantee = user_by_id(priv->grantee_id);<br>
> if (grantee == NULL)<br>
> - return;<br>
> + return 0;<br>
> /*<br>
> * Grant a role to a user only when privilege type is 'execute'<br>
> * and the role is specified.<br>
> @@ -3446,14 +3533,19 @@ grant_or_revoke(struct priv_def *priv)<br>
> if (priv->object_type == SC_ROLE && !(priv->access & ~PRIV_X)) {<br>
> struct user *role = user_by_id(priv->object_id);<br>
> if (role == NULL || role->def->type != SC_ROLE)<br>
> - return;<br>
> - if (priv->access)<br>
> - role_grant(grantee, role);<br>
> - else<br>
> - role_revoke(grantee, role);<br>
> + return 0;<br>
> + if (priv->access) {<br>
> + if (role_grant(grantee, role) != 0)<br>
> + return -1;<br>
> + } else {<br>
> + if (role_revoke(grantee, role) != 0)<br>
> + return -1;<br>
> + }<br>
> } else {<br>
> - priv_grant(grantee, priv);<br>
> + if (priv_grant(grantee, priv) != 0)<br>
> + return -1;<br>
> }<br>
> + return 0;<br>
> }<br>
> <br>
> /** A trigger called on rollback of grant. */<br>
> @@ -3463,9 +3555,11 @@ revoke_priv(struct trigger *trigger, void *event)<br>
> (void) event;<br>
> struct tuple *tuple = (struct tuple *)trigger->data;<br>
> struct priv_def priv;<br>
> - priv_def_create_from_tuple(&priv, tuple);<br>
> + if (priv_def_create_from_tuple(&priv, tuple) != 0)<br>
> + return -1;<br>
> priv.access = 0;<br>
> - grant_or_revoke(&priv);<br>
> + if (grant_or_revoke(&priv) != 0)<br>
> + return -1;<br>
> return 0;<br>
> }<br>
> <br>
> @@ -3476,8 +3570,10 @@ modify_priv(struct trigger *trigger, void *event)<br>
> (void) event;<br>
> struct tuple *tuple = (struct tuple *)trigger->data;<br>
> struct priv_def priv;<br>
> - priv_def_create_from_tuple(&priv, tuple);<br>
> - grant_or_revoke(&priv);<br>
> + if (priv_def_create_from_tuple(&priv, tuple) != 0)<br>
> + return -1;<br>
> + if (grant_or_revoke(&priv) != 0)<br>
> + return -1;<br>
> return 0;<br>
> }<br>
> <br>
> @@ -3495,27 +3591,42 @@ on_replace_dd_priv(struct trigger * /* trigger */,<br>
> void *event) struct priv_def priv;<br>
> <br>
> if (new_tuple != NULL && old_tuple == NULL) { /* grant */<br>
> - priv_def_create_from_tuple(&priv, new_tuple);<br>
> - priv_def_check(&priv, PRIV_GRANT);<br>
> - grant_or_revoke(&priv);<br>
> + if (priv_def_create_from_tuple(&priv, new_tuple) != 0)<br>
> + return -1;<br>
> + if (priv_def_check(&priv, PRIV_GRANT) != 0)<br>
> + return -1;<br>
> + if (grant_or_revoke(&priv) != 0)<br>
> + return -1;<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(revoke_priv, <br>
new_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> } else if (new_tuple == NULL) { /* revoke */<br>
> assert(old_tuple);<br>
> - priv_def_create_from_tuple(&priv, old_tuple);<br>
> - priv_def_check(&priv, PRIV_REVOKE);<br>
> + if (priv_def_create_from_tuple(&priv, old_tuple) != 0)<br>
> + return -1;<br>
> + if (priv_def_check(&priv, PRIV_REVOKE) != 0)<br>
> + return -1;<br>
It would be looking better if you combine such blocks into an or condition <br>
like:<br>
if (priv_def_create_from_tuple(&priv, old_tuple) != 0 ||<br>
priv_def_check(&priv, PRIV_REVOKE) != 0)<br>
return -1;<br>
<br>
> priv.access = 0;<br>
> - grant_or_revoke(&priv);<br>
> + if (grant_or_revoke(&priv) != 0)<br>
> + return -1;<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(modify_priv, <br>
old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> } else { /* modify */<br>
> - priv_def_create_from_tuple(&priv, new_tuple);<br>
> - priv_def_check(&priv, PRIV_GRANT);<br>
> - grant_or_revoke(&priv);<br>
> + if (priv_def_create_from_tuple(&priv, new_tuple) != 0)<br>
> + return -1;<br>
> + if (priv_def_check(&priv, PRIV_GRANT) != 0)<br>
> + return -1;<br>
> + if (grant_or_revoke(&priv) != 0)<br>
> + return -1;<br>
Please combine it into one condition here and other places<br>
> struct trigger *on_rollback =<br>
> txn_alter_trigger_new(modify_priv, <br>
old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> }<br>
> return 0;<br>
> @@ -3656,6 +3767,8 @@ on_replace_dd_cluster(struct trigger *trigger, void<br>
> *event) struct trigger *on_commit;<br>
> on_commit = <br>
txn_alter_trigger_new(register_replica,<br>
> <br>
new_tuple);<br>
> + if (on_commit == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> }<br>
> } else {<br>
> @@ -3672,6 +3785,8 @@ on_replace_dd_cluster(struct trigger *trigger, void<br>
> *event) struct trigger *on_commit;<br>
> on_commit = txn_alter_trigger_new(unregister_replica,<br>
> <br>
old_tuple);<br>
> + if (on_commit == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> }<br>
> return 0;<br>
> @@ -3794,10 +3909,13 @@ on_replace_dd_sequence(struct trigger * /* trigger<br>
> */, void *event) if (old_tuple == NULL && new_tuple != NULL) { /<br>
* INSERT<br>
> */<br>
> new_def = sequence_def_new_from_tuple(new_tuple,<br>
> <br>
ER_CREATE_SEQUENCE);<br>
> - access_check_ddl(new_def->name, new_def->id, new_def-<br>
>uid,<br>
> - SC_SEQUENCE, PRIV_C);<br>
> + if (access_check_ddl(new_def->name, new_def->id, <br>
new_def->uid,<br>
> + SC_SEQUENCE, PRIV_C) != 0)<br>
> + return -1;<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_create_sequence_rollback, NULL);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> seq = sequence_new_xc(new_def);<br>
> sequence_cache_insert(seq);<br>
> on_rollback->data = seq;<br>
> @@ -3807,8 +3925,9 @@ on_replace_dd_sequence(struct trigger * /* trigger */,<br>
> void *event) BOX_SEQUENCE_DATA_FIELD_ID);<br>
> seq = sequence_by_id(id);<br>
> assert(seq != NULL);<br>
> - access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> - SC_SEQUENCE, PRIV_D);<br>
> + if (access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> + SC_SEQUENCE, PRIV_D) != 0)<br>
> + return -1;<br>
> if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id)) {<br>
> diag_set(ClientError, ER_DROP_SEQUENCE,<br>
> seq->def->name, "the sequence <br>
has data");<br>
> @@ -3828,6 +3947,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */,<br>
> void *event) txn_alter_trigger_new(on_drop_sequence_commit, seq);<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_drop_sequence_rollback, seq);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> sequence_cache_delete(seq->def->id);<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> @@ -3836,12 +3957,15 @@ on_replace_dd_sequence(struct trigger * /* trigger<br>
> */, void *event) ER_ALTER_SEQUENCE);<br>
> seq = sequence_by_id(new_def->id);<br>
> assert(seq != NULL);<br>
> - access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> - SC_SEQUENCE, PRIV_A);<br>
> + if (access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> + SC_SEQUENCE, PRIV_A) != 0)<br>
> + return -1;<br>
> struct trigger *on_commit =<br>
> <br>
txn_alter_trigger_new(on_alter_sequence_commit, seq->def);<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_alter_sequence_rollback, seq->def);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> seq->def = new_def;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> @@ -3899,6 +4023,8 @@ on_replace_dd_sequence_data(struct trigger * /*<br>
> trigger */, void *event) */<br>
> struct trigger *on_rollback = txn_alter_trigger_new(<br>
> on_drop_sequence_data_rollback, <br>
old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> sequence_reset(seq);<br>
> }<br>
> @@ -4015,21 +4141,25 @@ on_replace_dd_space_sequence(struct trigger * /*<br>
> trigger */, void *event)<br>
> <br>
> /* Check we have the correct access type on the sequence. * */<br>
> if (is_generated || !stmt->new_tuple) {<br>
> - access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> - SC_SEQUENCE, priv_type);<br>
> + if (access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> + SC_SEQUENCE, priv_type) != 0)<br>
> + return -1;<br>
> } else {<br>
> /*<br>
> * In case user wants to attach an existing sequence,<br>
> * check that it has read and write access.<br>
> */<br>
> - access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> - SC_SEQUENCE, PRIV_R);<br>
> - access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> - SC_SEQUENCE, PRIV_W);<br>
> + if (access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> + SC_SEQUENCE, PRIV_R) != 0)<br>
> + return -1;<br>
> + if (access_check_ddl(seq->def->name, seq->def->id, seq-<br>
>def->uid,<br>
> + SC_SEQUENCE, PRIV_W) != 0)<br>
> + return -1;<br>
> }<br>
> /** Check we have alter access on space. */<br>
> - access_check_ddl(space->def->name, space->def->id, space->def-<br>
>uid,<br>
> - SC_SPACE, PRIV_A);<br>
> + if (access_check_ddl(space->def->name, space->def->id, space->def-<br>
>uid,<br>
> + SC_SPACE, PRIV_A) != 0)<br>
> + return -1;<br>
> <br>
> if (stmt->new_tuple != NULL) { /* <br>
INSERT, UPDATE */<br>
> char *sequence_path;<br>
> @@ -4052,6 +4182,8 @@ on_replace_dd_space_sequence(struct trigger * /*<br>
> trigger */, void *event) else<br>
> on_rollback = <br>
txn_alter_trigger_new(clear_space_sequence,<br>
> <br>
stmt->new_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> seq->is_generated = is_generated;<br>
> space->sequence = seq;<br>
> space->sequence_fieldno = sequence_fieldno;<br>
> @@ -4063,6 +4195,8 @@ on_replace_dd_space_sequence(struct trigger * /*<br>
> trigger */, void *event) struct trigger *on_rollback;<br>
> on_rollback = txn_alter_trigger_new(set_space_sequence,<br>
> stmt-<br>
>old_tuple);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> assert(space->sequence == seq);<br>
> seq->is_generated = false;<br>
> space->sequence = NULL;<br>
> @@ -4153,6 +4287,8 @@ on_replace_dd_trigger(struct trigger * /* trigger */,<br>
> void *event) struct trigger *on_rollback = txn_alter_trigger_new(NULL,<br>
> NULL); struct trigger *on_commit =<br>
> txn_alter_trigger_new(on_replace_trigger_commit, NULL);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> <br>
> if (old_tuple != NULL && new_tuple == NULL) {<br>
> /* DROP trigger. */<br>
> @@ -4655,6 +4791,8 @@ on_replace_dd_fk_constraint(struct trigger * /*<br>
> trigger*/, void *event) struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_create_fk_constraint_rollback,<br>
> fk);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> fk_constraint_set_mask(fk,<br>
> &parent_space-<br>
>fk_constraint_mask,<br>
> @@ -4673,10 +4811,14 @@ on_replace_dd_fk_constraint(struct trigger * /*<br>
> trigger*/, void *event) struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_replace_fk_constraint_rollback,<br>
> <br>
old_fk);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> struct trigger *on_commit =<br>
> <br>
txn_alter_trigger_new(on_drop_or_replace_fk_constraint_commit,<br>
> <br>
old_fk);<br>
> + if (on_commit == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> space_reset_fk_constraint_mask(child_space);<br>
> <br>
space_reset_fk_constraint_mask(parent_space);<br>
> @@ -4699,10 +4841,14 @@ on_replace_dd_fk_constraint(struct trigger * /*<br>
> trigger*/, void *event) struct trigger *on_commit =<br>
> <br>
txn_alter_trigger_new(on_drop_or_replace_fk_constraint_commit,<br>
> old_fk);<br>
> + if (on_commit == NULL)<br>
> + return -1;<br>
> txn_stmt_on_commit(stmt, on_commit);<br>
> struct trigger *on_rollback =<br>
> <br>
txn_alter_trigger_new(on_drop_fk_constraint_rollback,<br>
> old_fk);<br>
> + if (on_rollback == NULL)<br>
> + return -1;<br>
> txn_stmt_on_rollback(stmt, on_rollback);<br>
> space_reset_fk_constraint_mask(child_space);<br>
> space_reset_fk_constraint_mask(parent_space);<br>
> @@ -4830,6 +4976,8 @@ on_replace_dd_ck_constraint(struct trigger * /*<br>
> trigger*/, void *event) struct space *space =<br>
> space_cache_find_xc(space_id);<br>
> struct trigger *on_rollback = txn_alter_trigger_new(NULL, NULL);<br>
> struct trigger *on_commit = txn_alter_trigger_new(NULL, NULL);<br>
> + if (on_commit == NULL || on_rollback == NULL)<br>
> + return -1;<br>
> <br>
> if (new_tuple != NULL) {<br>
> bool is_deferred =<br>
> diff --git a/src/box/user.cc b/src/box/user.cc<br>
> index c46ff67d1..92a183c2a 100644<br>
> --- a/src/box/user.cc<br>
> +++ b/src/box/user.cc<br>
> @@ -180,18 +180,24 @@ user_destroy(struct user *user)<br>
> * Add a privilege definition to the list<br>
> * of effective privileges of a user.<br>
> */<br>
> -void<br>
> +int<br>
> user_grant_priv(struct user *user, struct priv_def *def)<br>
> {<br>
> struct priv_def *old = privset_search(&user->privs, def);<br>
> if (old == NULL) {<br>
> + size_t size = sizeof(struct priv_def);<br>
> old = (struct priv_def *)<br>
> - region_alloc_xc(&user->pool, sizeof(struct <br>
priv_def));<br>
> + region_alloc(&user->pool, size);<br>
> + if (old == NULL) {<br>
> + diag_set(OutOfMemory, size, "region", "new <br>
slab");<br>
> + return -1;<br>
> + }<br>
> *old = *def;<br>
> privset_insert(&user->privs, old);<br>
> } else {<br>
> old->access |= def->access;<br>
> }<br>
> + return 0;<br>
> }<br>
> <br>
> /**<br>
> @@ -305,11 +311,11 @@ user_set_effective_access(struct user *user)<br>
> /**<br>
> * Reload user privileges and re-grant them.<br>
> */<br>
> -static void<br>
> +static int<br>
> user_reload_privs(struct user *user)<br>
> {<br>
> if (user->is_dirty == false)<br>
> - return;<br>
> + return 0;<br>
> struct priv_def *priv;<br>
> /**<br>
> * Reset effective access of the user in the<br>
> @@ -326,26 +332,43 @@ user_reload_privs(struct user *user)<br>
> privset_new(&user->privs);<br>
> /* Load granted privs from _priv space. */<br>
> {<br>
> - struct space *space = space_cache_find_xc(BOX_PRIV_ID);<br>
> + struct space *space = space_cache_find(BOX_PRIV_ID);<br>
> + if (space == NULL)<br>
> + return -1;<br>
> char key[6];<br>
> /** Primary key - by user id */<br>
> - struct index *index = index_find_system_xc(space, 0);<br>
> + if (!space_is_memtx(space)) {<br>
> + diag_set(ClientError, ER_UNSUPPORTED,<br>
> + space->engine->name, "system <br>
data");<br>
> + return -1;<br>
> + }<br>
> + struct index *index = index_find(space, 0);<br>
> + if (index == NULL)<br>
> + return -1;<br>
> mp_encode_uint(key, user->def->uid);<br>
> <br>
> - struct iterator *it = index_create_iterator_xc(index, <br>
ITER_EQ,<br>
> + struct iterator *it = index_create_iterator(index, <br>
ITER_EQ,<br>
> <br>
key, 1);<br>
> + if (it == NULL)<br>
> + return -1;<br>
> IteratorGuard iter_guard(it);<br>
> <br>
> struct tuple *tuple;<br>
> - while ((tuple = iterator_next_xc(it)) != NULL) {<br>
> + if (iterator_next(it, &tuple) != 0)<br>
> + return -1;<br>
> + while (tuple != NULL) {<br>
> struct priv_def priv;<br>
> - priv_def_create_from_tuple(&priv, tuple);<br>
> + if (priv_def_create_from_tuple(&priv, tuple) <br>
!= 0)<br>
> + return -1;<br>
> /**<br>
> * Skip role grants, we're only<br>
> * interested in real objects.<br>
> */<br>
> if (priv.object_type != SC_ROLE || !<br>
(priv.access & PRIV_X))<br>
> - user_grant_priv(user, &priv);<br>
> + if (user_grant_priv(user, &priv) !<br>
= 0)<br>
> + return -1;<br>
> + if (iterator_next(it, &tuple) != 0)<br>
> + return -1;<br>
> }<br>
> }<br>
> {<br>
> @@ -358,12 +381,14 @@ user_reload_privs(struct user *user)<br>
> privset_ifirst(&role->privs, &it);<br>
> struct priv_def *def;<br>
> while ((def = privset_inext(&it))) {<br>
> - user_grant_priv(user, def);<br>
> + if (user_grant_priv(user, def) != <br>
0)<br>
> + return -1;<br>
> }<br>
> }<br>
> }<br>
> user_set_effective_access(user);<br>
> user->is_dirty = false;<br>
> + return 0;<br>
> }<br>
> <br>
> /** }}} */<br>
> @@ -559,7 +584,7 @@ user_cache_free()<br>
> <br>
> /** {{{ roles */<br>
> <br>
> -void<br>
> +int<br>
> role_check(struct user *grantee, struct user *role)<br>
> {<br>
> /*<br>
> @@ -592,16 +617,18 @@ role_check(struct user *grantee, struct user *role)<br>
> */<br>
> if (user_map_is_set(&transitive_closure,<br>
> role->auth_token)) {<br>
> - tnt_raise(ClientError, ER_ROLE_LOOP,<br>
> + diag_set(ClientError, ER_ROLE_LOOP,<br>
> role->def->name, grantee->def->name);<br>
> + return -1;<br>
> }<br>
> + return 0;<br>
> }<br>
> <br>
> /**<br>
> * Re-calculate effective grants of the linked subgraph<br>
> * this user/role is a part of.<br>
> */<br>
> -void<br>
> +int<br>
> rebuild_effective_grants(struct user *grantee)<br>
> {<br>
> /*<br>
> @@ -653,7 +680,8 @@ rebuild_effective_grants(struct user *grantee)<br>
> struct user_map indirect_edges = user-<br>
>roles;<br>
> user_map_minus(&indirect_edges, <br>
&transitive_closure);<br>
> if (user_map_is_empty(&indirect_edges)) {<br>
> - user_reload_privs(user);<br>
> + if (user_reload_privs(user) != 0)<br>
> + return -1;<br>
> user_map_union(&next_layer, <br>
&user->users);<br>
> } else {<br>
> /*<br>
> @@ -674,6 +702,7 @@ rebuild_effective_grants(struct user *grantee)<br>
> user_map_union(&transitive_closure, ¤t_layer);<br>
> current_layer = next_layer;<br>
> }<br>
> + return 0;<br>
> }<br>
> <br>
> <br>
> @@ -682,35 +711,41 @@ rebuild_effective_grants(struct user *grantee)<br>
> * Grant all effective privileges of the role to whoever<br>
> * this role was granted to.<br>
> */<br>
> -void<br>
> +int<br>
> role_grant(struct user *grantee, struct user *role)<br>
> {<br>
> user_map_set(&role->users, grantee->auth_token);<br>
> user_map_set(&grantee->roles, role->auth_token);<br>
> - rebuild_effective_grants(grantee);<br>
> + if (rebuild_effective_grants(grantee) != 0)<br>
> + return -1;<br>
> + return 0;<br>
> }<br>
> <br>
> /**<br>
> * Update the role dependencies graph.<br>
> * Rebuild effective privileges of the grantee.<br>
> */<br>
> -void<br>
> +int<br>
> role_revoke(struct user *grantee, struct user *role)<br>
> {<br>
> user_map_clear(&role->users, grantee->auth_token);<br>
> user_map_clear(&grantee->roles, role->auth_token);<br>
> - rebuild_effective_grants(grantee);<br>
> + if (rebuild_effective_grants(grantee) != 0)<br>
> + return -1;<br>
> + return 0;<br>
> }<br>
> <br>
> -void<br>
> +int<br>
> priv_grant(struct user *grantee, struct priv_def *priv)<br>
> {<br>
> struct access *object = access_find(priv->object_type, priv-<br>
>object_id);<br>
> if (object == NULL)<br>
> - return;<br>
> + return 0;<br>
> struct access *access = &object[grantee->auth_token];<br>
> access->granted = priv->access;<br>
> - rebuild_effective_grants(grantee);<br>
> + if (rebuild_effective_grants(grantee) != 0)<br>
> + return -1;<br>
> + return 0;<br>
> }<br>
> <br>
> /** }}} */<br>
> diff --git a/src/box/user.h b/src/box/user.h<br>
> index 527fb2e7c..ae2b73e55 100644<br>
> --- a/src/box/user.h<br>
> +++ b/src/box/user.h<br>
> @@ -144,16 +144,6 @@ user_cache_replace(struct user_def *user);<br>
> void<br>
> user_cache_delete(uint32_t uid);<br>
> <br>
> -/* Find a user by name. Used by authentication. */<br>
> -static inline struct user *<br>
> -user_find_xc(uint32_t uid)<br>
> -{<br>
> - struct user *user = user_find(uid);<br>
> - if (user == NULL)<br>
> - diag_raise();<br>
> - return user;<br>
> -}<br>
> -<br>
> static inline struct user *<br>
> user_find_by_name_xc(const char *name, uint32_t len)<br>
> {<br>
> @@ -178,19 +168,19 @@ user_cache_free();<br>
> * and no loop in the graph will occur when grantee gets<br>
> * a given role.<br>
> */<br>
> -void<br>
> +int<br>
> role_check(struct user *grantee, struct user *role);<br>
> <br>
> /**<br>
> * Grant a role to a user or another role.<br>
> */<br>
> -void<br>
> +int<br>
> role_grant(struct user *grantee, struct user *role);<br>
> <br>
> /**<br>
> * Revoke a role from a user or another role.<br>
> */<br>
> -void<br>
> +int<br>
> role_revoke(struct user *grantee, struct user *role);<br>
> <br>
> /**<br>
> @@ -198,10 +188,10 @@ role_revoke(struct user *grantee, struct user *role);<br>
> * and re-evaluate effective access of all users of this<br>
> * role if this role.<br>
> */<br>
> -void<br>
> +int<br>
> priv_grant(struct user *grantee, struct priv_def *priv);<br>
> <br>
> -void<br>
> +int<br>
> priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple);<br>
> <br>
> /* }}} */<br>
<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br></BODY></HTML>