[tarantool-patches] Re[2]: [PATCH v2] refactoring: remove exceptions from triggers

["Ilya Kosarev" <i.kosarev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 16167 bytes --]


From 1a55132c453e3f6f2fb7f73cd1f1a10ffd885722 Mon Sep 17 00:00:00 2001
From: Ilya Kosarev <i.kosarev@tarantool.org>
Date: Mon, 2 Sep 2019 14:37:19 +0300
Subject: [PATCH] refactoring: fix codestyle

switch .. case indentations are refactored.
Extra consequent whitespaces are replaced with tabs.
Suitable _xc function implemented.
Closes #4247
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4247-remove-exceptions-from-triggers 
Issue: https://github.com/tarantool/tarantool/issues/4247
---
src/box/alter.cc | 311 +++++++++++++++++++++--------------------
src/box/replication.cc | 134 +++++++++---------
src/box/user.cc | 9 +-
src/box/vy_scheduler.c | 3 +-
4 files changed, 232 insertions(+), 225 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 02d66741b..faa575559 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1330,30 +1330,30 @@ MoveIndex::rollback(struct alter_space *alter)
class ModifyIndex: public AlterSpaceOp
{
public:
- ModifyIndex(struct alter_space *alter,
- struct index *index, struct index_def *def)
- : AlterSpaceOp(alter), old_index(index),
- new_index(NULL), new_index_def(def) {
- if (new_index_def->iid == 0 &&
- key_part_cmp(new_index_def->key_def->parts,
- new_index_def->key_def->part_count,
- old_index->def->key_def->parts,
- old_index->def->key_def->part_count) != 0) {
- /*
- * Primary parts have been changed -
- * update secondary indexes.
- */
- alter->pk_def = new_index_def->key_def;
- }
- }
- struct index *old_index;
- struct index *new_index;
- struct index_def *new_index_def;
- virtual void alter_def(struct alter_space *alter);
- virtual void alter(struct alter_space *alter);
- virtual void commit(struct alter_space *alter, int64_t lsn);
- virtual void rollback(struct alter_space *alter);
- virtual ~ModifyIndex();
+ ModifyIndex(struct alter_space *alter,
+ struct index *index, struct index_def *def)
+ : AlterSpaceOp(alter), old_index(index),
+ new_index(NULL), new_index_def(def) {
+ if (new_index_def->iid == 0 &&
+ key_part_cmp(new_index_def->key_def->parts,
+ new_index_def->key_def->part_count,
+ old_index->def->key_def->parts,
+ old_index->def->key_def->part_count) != 0) {
+ /*
+ * Primary parts have been changed -
+ * update secondary indexes.
+ */
+ alter->pk_def = new_index_def->key_def;
+ }
+ }
+ struct index *old_index;
+ struct index *new_index;
+ struct index_def *new_index_def;
+ virtual void alter_def(struct alter_space *alter);
+ virtual void alter(struct alter_space *alter);
+ virtual void commit(struct alter_space *alter, int64_t lsn);
+ virtual void rollback(struct alter_space *alter);
+ virtual ~ModifyIndex();
};

/** Update the definition of the new space */
@@ -3111,16 +3111,16 @@ func_def_new_from_tuple(struct tuple *tuple)
uint32_t len;
const char *str = mp_decode_str(&exports, &len);
switch (STRN2ENUM(func_language, str, len)) {
- case FUNC_LANGUAGE_LUA:
- def->exports.lua = true;
- break;
- case FUNC_LANGUAGE_SQL:
- def->exports.sql = true;
- break;
- default:
- diag_set(ClientError, ER_CREATE_FUNCTION,
- def->name, "invalid exports value");
- return NULL;
+ case FUNC_LANGUAGE_LUA:
+ def->exports.lua = true;
+ break;
+ case FUNC_LANGUAGE_SQL:
+ def->exports.sql = true;
+ break;
+ default:
+ diag_set(ClientError, ER_CREATE_FUNCTION,
+ def->name, "invalid exports value");
+ return NULL;
}
}
const char *aggregate =
@@ -3278,9 +3278,10 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
}
/* Can't' drop a builtin function. */
if (old_func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
- tnt_raise(ClientError, ER_DROP_FUNCTION,
+ diag_set(ClientError, ER_DROP_FUNCTION,
(unsigned) old_func->def->uid,
"function is SQL built-in");
+ return -1;
}
struct trigger *on_commit =
txn_alter_trigger_new(on_drop_func_commit, old_func);
@@ -3579,17 +3580,17 @@ 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:
- if (tuple_field_u32(tuple,BOX_PRIV_FIELD_OBJECT_ID, &(priv->object_id)) != 0)
- return -1;
+ 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) {
diag_set(ClientError, ER_UNKNOWN_SCHEMA_OBJECT,
@@ -3631,121 +3632,121 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
priv->object_type, priv_type) != 0)
return -1;
switch (priv->object_type) {
- case SC_UNIVERSE:
- if (grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_UNIVERSE),
- name,
- grantor->def->name);
- return -1;
- }
- break;
- case SC_SPACE: {
- struct space *space = space_cache_find(priv->object_id);
- if (space == NULL)
- return -1;
- if (space->def->uid != grantor->def->uid &&
- grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_SPACE), name,
- grantor->def->name);
- return -1;
- }
- break;
+ case SC_UNIVERSE:
+ if (grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_UNIVERSE),
+ name,
+ grantor->def->name);
+ return -1;
}
- case SC_FUNCTION: {
- struct func *func = func_by_id(priv->object_id);
- if (func == NULL) {
- diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(priv->object_id));
- return -1;
- }
- if (func->def->uid != grantor->def->uid &&
- grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_FUNCTION), name,
- grantor->def->name);
- return -1;
- }
- break;
+ break;
+ case SC_SPACE: {
+ struct space *space = space_cache_find(priv->object_id);
+ if (space == NULL)
+ return -1;
+ if (space->def->uid != grantor->def->uid &&
+ grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_SPACE), name,
+ grantor->def->name);
+ return -1;
}
- case SC_SEQUENCE: {
- struct sequence *seq = sequence_by_id(priv->object_id);
- if (seq == NULL) {
- diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(priv->object_id));
- return -1;
- }
- if (seq->def->uid != grantor->def->uid &&
- grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_SEQUENCE), name,
- grantor->def->name);
- return -1;
- }
- break;
+ break;
+ }
+ case SC_FUNCTION: {
+ struct func *func = func_by_id(priv->object_id);
+ if (func == NULL) {
+ diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(priv->object_id));
+ return -1;
}
- case SC_ROLE: {
- struct user *role = user_by_id(priv->object_id);
- if (role == NULL || role->def->type != SC_ROLE) {
- diag_set(ClientError, ER_NO_SUCH_ROLE,
- role ? role->def->name :
- int2str(priv->object_id));
- return -1;
- }
- /*
- * Only the creator of the role can grant or revoke it.
- * Everyone can grant 'PUBLIC' role.
- */
- if (role->def->owner != grantor->def->uid &&
- grantor->def->uid != ADMIN &&
- (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_ROLE), name,
- grantor->def->name);
- return -1;
- }
- /* Not necessary to do during revoke, but who cares. */
- if (role_check(grantee, role) != 0)
- return -1;
- break;
+ if (func->def->uid != grantor->def->uid &&
+ grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_FUNCTION), name,
+ grantor->def->name);
+ return -1;
}
- case SC_USER: {
- struct user *user = user_by_id(priv->object_id);
- if (user == NULL || user->def->type != SC_USER) {
- diag_set(ClientError, ER_NO_SUCH_USER,
- user ? user->def->name :
- int2str(priv->object_id));
- return -1;
- }
- if (user->def->owner != grantor->def->uid &&
- grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_USER), name,
- grantor->def->name);
- return -1;
- }
- break;
+ break;
+ }
+ case SC_SEQUENCE: {
+ struct sequence *seq = sequence_by_id(priv->object_id);
+ if (seq == NULL) {
+ diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(priv->object_id));
+ return -1;
}
- case SC_ENTITY_SPACE:
- case SC_ENTITY_FUNCTION:
- case SC_ENTITY_SEQUENCE:
- case SC_ENTITY_ROLE:
- case SC_ENTITY_USER: {
- /* Only admin may grant privileges on an entire entity. */
- if (grantor->def->uid != ADMIN) {
- diag_set(AccessDeniedError, priv_name(priv_type),
- schema_object_name(priv->object_type), name,
- grantor->def->name);
- return -1;
- }
+ if (seq->def->uid != grantor->def->uid &&
+ grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_SEQUENCE), name,
+ grantor->def->name);
+ return -1;
}
- default:
- break;
+ break;
+ }
+ case SC_ROLE: {
+ struct user *role = user_by_id(priv->object_id);
+ if (role == NULL || role->def->type != SC_ROLE) {
+ diag_set(ClientError, ER_NO_SUCH_ROLE,
+ role ? role->def->name :
+ int2str(priv->object_id));
+ return -1;
+ }
+ /*
+ * Only the creator of the role can grant or revoke it.
+ * Everyone can grant 'PUBLIC' role.
+ */
+ if (role->def->owner != grantor->def->uid &&
+ grantor->def->uid != ADMIN &&
+ (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_ROLE), name,
+ grantor->def->name);
+ return -1;
+ }
+ /* Not necessary to do during revoke, but who cares. */
+ if (role_check(grantee, role) != 0)
+ return -1;
+ break;
+ }
+ case SC_USER: {
+ struct user *user = user_by_id(priv->object_id);
+ if (user == NULL || user->def->type != SC_USER) {
+ diag_set(ClientError, ER_NO_SUCH_USER,
+ user ? user->def->name :
+ int2str(priv->object_id));
+ return -1;
+ }
+ if (user->def->owner != grantor->def->uid &&
+ grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_USER), name,
+ grantor->def->name);
+ return -1;
+ }
+ break;
+ }
+ case SC_ENTITY_SPACE:
+ case SC_ENTITY_FUNCTION:
+ case SC_ENTITY_SEQUENCE:
+ case SC_ENTITY_ROLE:
+ case SC_ENTITY_USER: {
+ /* Only admin may grant privileges on an entire entity. */
+ if (grantor->def->uid != ADMIN) {
+ diag_set(AccessDeniedError, priv_name(priv_type),
+ schema_object_name(priv->object_type), name,
+ grantor->def->name);
+ return -1;
+ }
+ }
+ default:
+ break;
}
if (priv->access == 0) {
diag_set(ClientError, ER_GRANT,
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 68bc1b064..bd3d4c23f 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -386,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)
@@ -415,43 +415,43 @@ replica_on_applier_state_f(struct trigger *trigger, void *event)
struct replica *replica = container_of(trigger,
struct replica, on_applier_state);
switch (replica->applier->state) {
- case APPLIER_INITIAL_JOIN:
- replicaset.is_joining = true;
- break;
- case APPLIER_JOINED:
- replicaset.is_joining = false;
- break;
- case APPLIER_CONNECTED:
- try {
- if (tt_uuid_is_nil(&replica->uuid))
- replica_on_applier_connect(replica);
- else
- replica_on_applier_reconnect(replica);
- } catch (Exception *e) {
- return -1;
- }
- break;
- case APPLIER_LOADING:
- case APPLIER_DISCONNECTED:
- replica_on_applier_disconnect(replica);
- break;
- case APPLIER_FOLLOW:
- replica_on_applier_sync(replica);
- break;
- case APPLIER_OFF:
- /*
- * Connection to self, duplicate connection
- * to the same master, or the applier fiber
- * has been cancelled. Assume synced.
- */
- replica_on_applier_sync(replica);
- break;
- case APPLIER_STOPPED:
- /* Unrecoverable error. */
- replica_on_applier_disconnect(replica);
- break;
- default:
- break;
+ case APPLIER_INITIAL_JOIN:
+ replicaset.is_joining = true;
+ break;
+ case APPLIER_JOINED:
+ replicaset.is_joining = false;
+ break;
+ case APPLIER_CONNECTED:
+ try {
+ if (tt_uuid_is_nil(&replica->uuid))
+ replica_on_applier_connect(replica);
+ else
+ replica_on_applier_reconnect(replica);
+ } catch (Exception *e) {
+ return -1;
+ }
+ break;
+ case APPLIER_LOADING:
+ case APPLIER_DISCONNECTED:
+ replica_on_applier_disconnect(replica);
+ break;
+ case APPLIER_FOLLOW:
+ replica_on_applier_sync(replica);
+ break;
+ case APPLIER_OFF:
+ /*
+ * Connection to self, duplicate connection
+ * to the same master, or the applier fiber
+ * has been cancelled. Assume synced.
+ */
+ replica_on_applier_sync(replica);
+ break;
+ case APPLIER_STOPPED:
+ /* Unrecoverable error. */
+ replica_on_applier_disconnect(replica);
+ break;
+ default:
+ break;
}
fiber_cond_signal(&replicaset.applier.cond);
return 0;
@@ -472,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 */
@@ -596,15 +596,15 @@ applier_on_connect_f(struct trigger *trigger, void *event)
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);
diff --git a/src/box/user.cc b/src/box/user.cc
index 50614c6f2..366ebcd72 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -302,6 +302,12 @@ user_set_effective_access(struct user *user)
}
}

+static void priv_def_create_from_tuple_xc(struct priv_def *priv, struct tuple *tuple)
+{
+ if (priv_def_create_from_tuple(priv, tuple) != 0)
+ diag_raise();
+}
+
/**
* Reload user privileges and re-grant them.
*/
@@ -339,8 +345,7 @@ user_reload_privs(struct user *user)
struct tuple *tuple;
while ((tuple = iterator_next_xc(it)) != NULL) {
struct priv_def priv;
- if (priv_def_create_from_tuple(&priv, tuple) != 0)
- diag_raise();
+ priv_def_create_from_tuple_xc(&priv, tuple);
/**
* Skip role grants, we're only
* interested in real objects.
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index ee361c31f..86bed8013 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -510,7 +510,7 @@ vy_scheduler_reset_stat(struct vy_scheduler *scheduler)
stat->compaction_output = 0;
}

-static void
+static int
vy_scheduler_on_delete_lsm(struct trigger *trigger, void *event)
{
struct vy_lsm *lsm = event;
@@ -521,6 +521,7 @@ vy_scheduler_on_delete_lsm(struct trigger *trigger, void *event)
vy_compaction_heap_delete(&scheduler->compaction_heap, lsm);
trigger_clear(trigger);
free(trigger);
+ return 0;
}

int
-- 
2.17.1

[-- Attachment #2: Type: text/html, Size: 19214 bytes --]