[Tarantool-patches] [PATCH 5/5] Apply Clang formatter
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Nov 8 18:11:25 MSK 2020
See 268 comments below.
Many of them are about the same problems, but still I skipped a lot more due
to the diff being too huge (even though you easily could split it into parts).
In total: the formatter turns the code into shit. You probably would see that,
if you opened your own patch before sending it. From what I saw, you didn't
look at it. Maybe looked at a couple of diff hunks in the beginning.
As a proof, to those who read it, I propose to look at these comments: 20, 51,
79, 111, 152, 153, 164, 259. It is not all of them, but just a few. Proving that
Kirill does not care. This whole patch is just ticking of the boxes in the
planning table, not related to making things actually better for us.
Also I stick to the points I left in the previous review, on which you didn't
respond properly except that "it is discussed" somewhere without any artefacts
of course. It just proves you have nothing to say.
Besides, I understand, that you don't care and already decided to push it, without
listening to anyone. So I will repeat: if you commit it like this - I quit. At
least for month, or until the end of the quarter. I won't review shit, I won't do
shit, won't answer in the chats. Or more than for quarter, if I will like spending
free time on something except tarantool. Otherwise you don't hear me, it seems.
Maybe that will work.
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index af7ecf463..b2550ed70 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -366,11 +370,13 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space)
> index_def_set_func(index_def, func);
> }
> if (index_def->iid == 0 && space->sequence != NULL)
> - if (index_def_check_sequence(index_def, space->sequence_fieldno,
> - space->sequence_path,
> - space->sequence_path != NULL ?
> - strlen(space->sequence_path) : 0,
> - space_name(space)) != 0)
> + if (index_def_check_sequence(
> + index_def, space->sequence_fieldno,
> + space->sequence_path,
> + space->sequence_path != NULL ?
> + strlen(space->sequence_path) :
> + 0,
> + space_name(space)) != 0)
1. The previous formatting was fine, and now it looks worse. Why did it
wrap the arguments so early? And why did it add 14 chars on the lines
with "strlen()" and "0,". I would understand +1 tab, but 14 chars (1 tab
and 6 spaces) looks ridiculous. It looks not confirming to our style at all,
and is just ugly.
> @@ -445,15 +451,14 @@ field_def_decode(struct field_def *field, const char **data,
> fieldno + TUPLE_INDEX_BASE, region,
> true) != 0)
> return -1;
> - if (is_action_missing &&
> - key_len == action_literal_len &&
> + if (is_action_missing && key_len == action_literal_len &&
> memcmp(key, "nullable_action", action_literal_len) == 0)
> is_action_missing = false;
> }
> if (is_action_missing) {
> field->nullable_action = field->is_nullable ?
> - ON_CONFLICT_ACTION_NONE
> - : ON_CONFLICT_ACTION_DEFAULT;
> + ON_CONFLICT_ACTION_NONE :
> + ON_CONFLICT_ACTION_DEFAULT;
2. The same. Why +14 chars?
> @@ -1569,25 +1565,26 @@ RebuildIndex::~RebuildIndex()
> * drop the old index data and rebuild index from by reading the
> * primary key.
> */
> -class RebuildFuncIndex: public RebuildIndex
> -{
> - struct index_def *
> - func_index_def_new(struct index_def *index_def, struct func *func)
> +class RebuildFuncIndex: public RebuildIndex {
> + struct index_def *func_index_def_new(struct index_def *index_def,
> + struct func *func)
3. We don't declare return type on the same line as a function name. If
there are such places, they violate the style, and probably are just old
code.
> {
> struct index_def *new_index_def = index_def_dup_xc(index_def);
> index_def_set_func(new_index_def, func);
> return new_index_def;
> }
> @@ -2298,37 +2296,39 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
> }
> } else if (new_tuple == NULL) { /* DELETE */
> if (access_check_ddl(old_space->def->name, old_space->def->id,
> - old_space->def->uid, SC_SPACE, PRIV_D) != 0)
> + old_space->def->uid, SC_SPACE,
> + PRIV_D) != 0)
> return -1;
> /* Verify that the space is empty (has no indexes) */
> if (old_space->index_count) {
> diag_set(ClientError, ER_DROP_SPACE,
> - space_name(old_space),
> - "the space has indexes");
> + space_name(old_space),
> + "the space has indexes");
> return -1;
> }
> bool out;
> - if (schema_find_grants("space", old_space->def->id, &out) != 0) {
> + if (schema_find_grants("space", old_space->def->id, &out) !=
> + 0) {
4. This is ugly.
> return -1;
> }
> @@ -2404,47 +2401,48 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
> "view can not be altered");
> return -1;
> }
> - struct space_def *def =
> - space_def_new_from_tuple(new_tuple, ER_ALTER_SPACE,
> - region);
> + struct space_def *def = space_def_new_from_tuple(new_tuple,
> + ER_ALTER_SPACE,
> + region);
> if (def == NULL)
> return -1;
> - auto def_guard =
> - make_scoped_guard([=] { space_def_delete(def); });
> + auto def_guard = make_scoped_guard(
> + [=] { space_def_delete(def); });
> if (access_check_ddl(def->name, def->id, def->uid, SC_SPACE,
> - PRIV_A) != 0)
> + PRIV_A) != 0)
> return -1;
> if (def->id != space_id(old_space)) {
> diag_set(ClientError, ER_ALTER_SPACE,
> - space_name(old_space),
> - "space id is immutable");
> + space_name(old_space),
> + "space id is immutable");
> return -1;
> }
> - if (strcmp(def->engine_name, old_space->def->engine_name) != 0) {
> + if (strcmp(def->engine_name, old_space->def->engine_name) !=
> + 0) {
5. Ugly.
> diag_set(ClientError, ER_ALTER_SPACE,
> - space_name(old_space),
> - "can not change space engine");
> + space_name(old_space),
> + "can not change space engine");
> return -1;
> }
> @@ -2678,18 +2673,20 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
> if (old_index == NULL && new_tuple != NULL) {
> if (alter_space_move_indexes(alter, 0, iid))
> return -1;
> - struct index_def *def =
> - index_def_new_from_tuple(new_tuple, old_space);
> + struct index_def *def = index_def_new_from_tuple(new_tuple,
> + old_space);
> if (def == NULL)
> return -1;
> index_def_update_optionality(def, alter->new_min_field_count);
> try {
> if (def->opts.is_unique) {
> - (void) new CreateConstraintID(
> - alter, iid == 0 ? CONSTRAINT_TYPE_PK :
> - CONSTRAINT_TYPE_UNIQUE, def->name);
> + (void)new CreateConstraintID(
> + alter,
> + iid == 0 ? CONSTRAINT_TYPE_PK :
> + CONSTRAINT_TYPE_UNIQUE,
6. Why +17 chars on the last line above? Ugly and the number of
chars is ridiculous. It is not even due to an alignment. Looks
absolutely random.
> + def->name);
> }
> @@ -3525,32 +3523,34 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
> * who created it or a superuser.
> */
> if (access_check_ddl(old_func->def->name, fid, uid, SC_FUNCTION,
> - PRIV_D) != 0)
> + PRIV_D) != 0)
> return -1;
> /* Can only delete func if it has no grants. */
> bool out;
> - if (schema_find_grants("function", old_func->def->fid, &out) != 0) {
> + if (schema_find_grants("function", old_func->def->fid, &out) !=
> + 0) {
7. Ugly.
> return -1;
> }
> @@ -3602,22 +3602,23 @@ coll_id_def_new_from_tuple(struct tuple *tuple, struct coll_id_def *def)
> def->name_len = name_len;
> if (name_len > BOX_NAME_MAX) {
> diag_set(ClientError, ER_CANT_CREATE_COLLATION,
> - "collation name is too long");
> + "collation name is too long");
> return -1;
> }
> if (identifier_check(def->name, name_len) != 0)
> return -1;
> - if (tuple_field_u32(tuple, BOX_COLLATION_FIELD_UID, &(def->owner_id)) != 0)
> + if (tuple_field_u32(tuple, BOX_COLLATION_FIELD_UID, &(def->owner_id)) !=
> + 0)
8. Ugly.
> @@ -3754,7 +3755,8 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
> * identifier.
> */
> uint32_t out;
> - if (tuple_field_u32(old_tuple, BOX_COLLATION_FIELD_ID, &out) != 0)
> + if (tuple_field_u32(old_tuple, BOX_COLLATION_FIELD_ID, &out) !=
> + 0)
9. Ugly.
> return -1;
> int32_t old_id = out;
> @@ -3822,12 +3824,14 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
> int
> priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
> {
> - if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ID, &(priv->grantor_id)) != 0 ||
> - tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &(priv->grantee_id)) != 0)
> + if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ID, &(priv->grantor_id)) !=
> + 0 ||
> + tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &(priv->grantee_id)) !=
> + 0)
10. Double ugly.
> return -1;
>
> @@ -3900,70 +3904,63 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
> - case SC_ROLE:
> - {
> + 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));
> + role ? role->def->name :
> + int2str(priv->object_id));
11. This time +13 chars on the second line. What is it going to be
next time? +12 or +16? Random as it is.
> return -1;
> }
> @@ -3984,21 +3980,19 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
> return -1;
> break;
> }
> - case SC_USER:
> - {
> + 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));
> + user ? user->def->name :
> + int2str(priv->object_id));
12. +13 ridiculously random chars.
> @@ -4168,7 +4161,8 @@ on_replace_dd_schema(struct trigger * /* trigger */, void *event)
> return -1;
> }
> tt_uuid uu;
> - if (tuple_field_uuid(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu) != 0)
> + if (tuple_field_uuid(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu) !=
> + 0)
13. Ugly.
> return -1;
> REPLICASET_UUID = uu;
> say_info("cluster uuid %s", tt_uuid_str(&uu));
> @@ -4348,11 +4344,14 @@ sequence_def_new_from_tuple(struct tuple *tuple, uint32_t errcode)
> return NULL;
> if (tuple_field_i64(tuple, BOX_SEQUENCE_FIELD_MAX, &(def->max)) != 0)
> return NULL;
> - if (tuple_field_i64(tuple, BOX_SEQUENCE_FIELD_START, &(def->start)) != 0)
> + if (tuple_field_i64(tuple, BOX_SEQUENCE_FIELD_START, &(def->start)) !=
> + 0)
> return NULL;
> - if (tuple_field_i64(tuple, BOX_SEQUENCE_FIELD_CACHE, &(def->cache)) != 0)
> + if (tuple_field_i64(tuple, BOX_SEQUENCE_FIELD_CACHE, &(def->cache)) !=
> + 0)
> return NULL;
> - if (tuple_field_bool(tuple, BOX_SEQUENCE_FIELD_CYCLE, &(def->cycle)) != 0)
> + if (tuple_field_bool(tuple, BOX_SEQUENCE_FIELD_CYCLE, &(def->cycle)) !=
> + 0)
14. Triple ugly.
> return NULL;
> if (def->step == 0) {
> diag_set(ClientError, errcode, def->name,
> @@ -4705,9 +4704,10 @@ clear_space_sequence(struct trigger *trigger, void * /* event */)
> static int
> on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
> {
> - struct txn *txn = (struct txn *) event;
> + struct txn *txn = (struct txn *)event;
> struct txn_stmt *stmt = txn_current_stmt(txn);
> - struct tuple *tuple = stmt->new_tuple ? stmt->new_tuple : stmt->old_tuple;
> + struct tuple *tuple = stmt->new_tuple ? stmt->new_tuple :
> + stmt->old_tuple;
15. Ok, I was wrong. It is not 12 or 16. This time it is +24 chars. Ugly and
random.
> @@ -4922,34 +4922,33 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
> } else {
> /* INSERT, REPLACE trigger. */
> uint32_t trigger_name_len;
> - const char *trigger_name_src = tuple_field_str(new_tuple,
> - BOX_TRIGGER_FIELD_NAME, &trigger_name_len);
> + const char *trigger_name_src = tuple_field_str(
> + new_tuple, BOX_TRIGGER_FIELD_NAME, &trigger_name_len);
> if (trigger_name_src == NULL)
> return -1;
> - const char *space_opts = tuple_field_with_type(new_tuple,
> - BOX_TRIGGER_FIELD_OPTS,MP_MAP);
> + const char *space_opts = tuple_field_with_type(
> + new_tuple, BOX_TRIGGER_FIELD_OPTS, MP_MAP);
> if (space_opts == NULL)
> return -1;
> struct space_opts opts;
> struct region *region = &fiber()->gc;
> if (space_opts_decode(&opts, space_opts, region) != 0)
> return -1;
> - struct sql_trigger *new_trigger =
> - sql_trigger_compile(sql_get(), opts.sql);
> + struct sql_trigger *new_trigger = sql_trigger_compile(sql_get(),
> + opts.sql);
> if (new_trigger == NULL)
> return -1;
>
> - auto new_trigger_guard = make_scoped_guard([=] {
> - sql_trigger_delete(sql_get(), new_trigger);
> - });
> + auto new_trigger_guard = make_scoped_guard(
> + [=] { sql_trigger_delete(sql_get(), new_trigger); });
>
> const char *trigger_name = sql_trigger_name(new_trigger);
> if (strlen(trigger_name) != trigger_name_len ||
> - memcmp(trigger_name_src, trigger_name,
> - trigger_name_len) != 0) {
> + memcmp(trigger_name_src, trigger_name, trigger_name_len) !=
> + 0) {
16. Ugly.
> diag_set(ClientError, ER_SQL_EXECUTE,
> - "trigger name does not match extracted "
> - "from SQL");
> + "trigger name does not match extracted "
> + "from SQL");
> return -1;
> }
> @@ -5402,10 +5400,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
> continue;
> uint32_t j;
> for (j = 0; j < fk_def->field_count; ++j) {
> - if (key_def_find_by_fieldno(idx->def->key_def,
> - fk_def->links[j].
> - parent_field) ==
> - NULL)
> + if (key_def_find_by_fieldno(
> + idx->def->key_def,
> + fk_def->links[j].parent_field) ==
> + NULL)
17. Ugly.
> @@ -5870,68 +5866,58 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
> return 0;
> }
>
> -struct trigger alter_space_on_replace_space = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_space, NULL, NULL
> -};
> +struct trigger alter_space_on_replace_space = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_space, NULL,
> + NULL };
>
> -struct trigger alter_space_on_replace_index = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_index, NULL, NULL
> -};
> +struct trigger alter_space_on_replace_index = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_index, NULL,
> + NULL };
>
> -struct trigger on_replace_truncate = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_truncate, NULL, NULL
> -};
> +struct trigger on_replace_truncate = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_truncate, NULL, NULL };
>
> -struct trigger on_replace_schema = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_schema, NULL, NULL
> -};
> +struct trigger on_replace_schema = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_schema, NULL, NULL };
>
> -struct trigger on_replace_user = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_user, NULL, NULL
> -};
> +struct trigger on_replace_user = { RLIST_LINK_INITIALIZER, on_replace_dd_user,
> + NULL, NULL };
>
> -struct trigger on_replace_func = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_func, NULL, NULL
> -};
> +struct trigger on_replace_func = { RLIST_LINK_INITIALIZER, on_replace_dd_func,
> + NULL, NULL };
>
> -struct trigger on_replace_collation = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_collation, NULL, NULL
> -};
> +struct trigger on_replace_collation = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_collation, NULL, NULL };
>
> -struct trigger on_replace_priv = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_priv, NULL, NULL
> -};
> +struct trigger on_replace_priv = { RLIST_LINK_INITIALIZER, on_replace_dd_priv,
> + NULL, NULL };
>
> -struct trigger on_replace_cluster = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_cluster, NULL, NULL
> -};
> +struct trigger on_replace_cluster = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_cluster, NULL, NULL };
>
> -struct trigger on_replace_sequence = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_sequence, NULL, NULL
> -};
> +struct trigger on_replace_sequence = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_sequence, NULL, NULL };
>
> -struct trigger on_replace_sequence_data = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_sequence_data, NULL, NULL
> -};
> +struct trigger on_replace_sequence_data = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_sequence_data, NULL,
> + NULL };
>
> -struct trigger on_replace_space_sequence = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_space_sequence, NULL, NULL
> -};
> +struct trigger on_replace_space_sequence = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_space_sequence, NULL,
> + NULL };
>
> -struct trigger on_replace_trigger = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_trigger, NULL, NULL
> -};
> +struct trigger on_replace_trigger = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_trigger, NULL, NULL };
>
> -struct trigger on_replace_fk_constraint = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_fk_constraint, NULL, NULL
> -};
> +struct trigger on_replace_fk_constraint = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_fk_constraint, NULL,
> + NULL };
>
> -struct trigger on_replace_ck_constraint = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_ck_constraint, NULL, NULL
> -};
> +struct trigger on_replace_ck_constraint = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_ck_constraint, NULL,
> + NULL };
>
> -struct trigger on_replace_func_index = {
> - RLIST_LINK_INITIALIZER, on_replace_dd_func_index, NULL, NULL
> -};
> +struct trigger on_replace_func_index = { RLIST_LINK_INITIALIZER,
> + on_replace_dd_func_index, NULL, NULL };
18. This whole hunk is a praise to ugliness and bad readability.
> /* vim: set foldmethod=marker */
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 7686d6cbc..e0aa84dcb 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -185,9 +184,8 @@ applier_writer_f(va_list ap)
> struct xrow_header xrow;
> xrow_encode_vclock(&xrow, &replicaset.vclock);
> coio_write_xrow(&io, &xrow);
> - ERROR_INJECT(ERRINJ_APPLIER_SLOW_ACK, {
> - fiber_sleep(0.01);
> - });
> + ERROR_INJECT(ERRINJ_APPLIER_SLOW_ACK,
> + { fiber_sleep(0.01); });
19. Injections were supposed to be like an 'if' block. Not like a
lambda. It looks worse now.
> /*
> * Even if new ACK is requested during the
> * write, don't send it again right away.
> @@ -385,16 +383,17 @@ applier_connect(struct applier *applier)
> coio_read_xrow(coio, ibuf, &row);
> if (row.type == IPROTO_OK) {
> xrow_decode_ballot_xc(&row, &applier->ballot);
> - } else try {
> - xrow_decode_error_xc(&row);
> - } catch (ClientError *e) {
> - if (e->errcode() != ER_UNKNOWN_REQUEST_TYPE)
> - e->raise();
> - /*
> + } else
> + try {
> + xrow_decode_error_xc(&row);
> + } catch (ClientError *e) {
> + if (e->errcode() != ER_UNKNOWN_REQUEST_TYPE)
> + e->raise();
> + /*
> * Master isn't aware of IPROTO_VOTE request.
> * It's OK - we can proceed without it.
> */
20. The formatter clearly has failed its job here, twice.
Firstly, if 'if' has {}, 'else' also should have {}.
Secondly, the comment is misaligned as fuck. And the formatter
didn't do anything.
> @@ -720,8 +720,8 @@ applier_read_tx(struct applier *applier, struct stailq *rows)
> }
> stailq_add_tail(rows, &tx_row->next);
>
> - } while (!stailq_last_entry(rows, struct applier_tx_row,
> - next)->row.is_commit);
> + } while (!stailq_last_entry(rows, struct applier_tx_row, next)
> + ->row.is_commit);
21. Did you see this? Is it really better now according to you?
We are not compilers. We have human eyes. How are we supposed to
read such code?
> @@ -929,10 +928,10 @@ applier_apply_tx(struct applier *applier, struct stailq *rows)
> * that belong to the same server id.
> */
> struct latch *latch = (replica ? &replica->order_latch :
> - &replicaset.applier.order_latch);
> + &replicaset.applier.order_latch);
22. This time it is +17 chars. Can't wait to see what will the random
return next time.
> latch_lock(latch);
> - if (vclock_get(&replicaset.applier.vclock,
> - last_row->replica_id) >= last_row->lsn) {
> + if (vclock_get(&replicaset.applier.vclock, last_row->replica_id) >=
> + last_row->lsn) {
> latch_unlock(latch);
> return 0;
> } else if (vclock_get(&replicaset.applier.vclock,
> @@ -944,9 +943,9 @@ applier_apply_tx(struct applier *applier, struct stailq *rows)
> */
> struct xrow_header *tmp;
> while (true) {
> - tmp = &stailq_first_entry(rows,
> - struct applier_tx_row,
> - next)->row;
> + tmp = &stailq_first_entry(rows, struct applier_tx_row,
> + next)
> + ->row;
23. King of ugly.
> if (tmp->lsn <= vclock_get(&replicaset.applier.vclock,
> tmp->replica_id)) {
> stailq_shift(rows);
> @@ -981,7 +980,8 @@ applier_apply_tx(struct applier *applier, struct stailq *rows)
> latch_unlock(latch);
> return -1;
> }
> - stailq_foreach_entry(item, rows, next) {
> + stailq_foreach_entry(item, rows, next)
> + {
24. This is a 'for' cycle. It is not supposed to be formatted so.
> struct xrow_header *row = &item->row;
> int res = apply_row(row);
> if (res != 0) {
> @@ -1254,14 +1256,14 @@ applier_subscribe(struct applier *applier)
> * and check applier state.
> */
> struct xrow_header *first_row =
> - &stailq_first_entry(&rows, struct applier_tx_row,
> - next)->row;
> + &stailq_first_entry(&rows, struct applier_tx_row, next)
> + ->row;
25. Ugly.
> raft_process_heartbeat(applier->instance_id);
> if (first_row->lsn == 0) {
> if (unlikely(iproto_type_is_raft_request(
> - first_row->type))) {
> - if (applier_handle_raft(applier,
> - first_row) != 0)
> + first_row->type))) {
> + if (applier_handle_raft(applier, first_row) !=
> + 0)
26. Ugly.
> diag_raise();
> }
> @@ -1448,7 +1451,7 @@ applier_f(va_list ap)
> *
> * See: https://github.com/tarantool/tarantool/issues/136
> */
> -reconnect:
> + reconnect:
27. Labels are visible everywhere inside a function, and should not
have indentation.
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 18568df3b..52b2b1070 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -1033,8 +1031,9 @@ box_clear_synchro_queue(bool try_wait)
> if (!txn_limbo_is_empty(&txn_limbo)) {
> int64_t lsns[VCLOCK_MAX];
> int len = 0;
> - const struct vclock *vclock;
> - replicaset_foreach(replica) {
> + const struct vclock *vclock;
> + replicaset_foreach(replica)
> + {
28. This is a for loop and is supposed to be formatted exactly like a
for loop.
> @@ -1598,16 +1595,15 @@ sequence_data_update(uint32_t seq_id, int64_t value)
> char *tuple_buf_end = tuple_buf;
> tuple_buf_end = mp_encode_array(tuple_buf_end, 2);
> tuple_buf_end = mp_encode_uint(tuple_buf_end, seq_id);
> - tuple_buf_end = (value < 0 ?
> - mp_encode_int(tuple_buf_end, value) :
> - mp_encode_uint(tuple_buf_end, value));
> + tuple_buf_end = (value < 0 ? mp_encode_int(tuple_buf_end, value) :
> + mp_encode_uint(tuple_buf_end, value));
29. Finally, +19 chars. Lets see what will randomly happen next time.
> assert(tuple_buf_end < tuple_buf + tuple_buf_size);
>
> struct credentials *orig_credentials = effective_user();
> fiber_set_user(fiber(), &admin_credentials);
>
> - int rc = box_replace(BOX_SEQUENCE_DATA_ID,
> - tuple_buf, tuple_buf_end, NULL);
> + int rc = box_replace(BOX_SEQUENCE_DATA_ID, tuple_buf, tuple_buf_end,
> + NULL);
> @@ -2371,15 +2367,15 @@ bootstrap_from_master(struct replica *master)
>
> assert(!tt_uuid_is_nil(&INSTANCE_UUID));
> enum applier_state wait_state = replication_anon ?
> - APPLIER_FETCH_SNAPSHOT :
> - APPLIER_INITIAL_JOIN;
> + APPLIER_FETCH_SNAPSHOT :
> + APPLIER_INITIAL_JOIN;
30. Cursed. Here and below.
> applier_resume_to_state(applier, wait_state, TIMEOUT_INFINITY);
> /*
> * Process initial data (snapshot or dirty disk data).
> */
> engine_begin_initial_recovery_xc(NULL);
> wait_state = replication_anon ? APPLIER_FETCHED_SNAPSHOT :
> - APPLIER_FINAL_JOIN;
> + APPLIER_FINAL_JOIN;
> applier_resume_to_state(applier, wait_state, TIMEOUT_INFINITY);
>
> @@ -2874,7 +2871,8 @@ box_backup_start(int checkpoint_idx, box_backup_cb cb, void *cb_arg)
> return -1;
> }
> struct gc_checkpoint *checkpoint;
> - gc_foreach_checkpoint_reverse(checkpoint) {
> + gc_foreach_checkpoint_reverse(checkpoint)
> + {
31. It was correct before, because it is a for loop.
> if (checkpoint_idx-- == 0)
> break;
> }
> diff --git a/src/box/coll_id_def.c b/src/box/coll_id_def.c
> index 9fe0cda8c..d518eadf6 100644
> --- a/src/box/coll_id_def.c
> +++ b/src/box/coll_id_def.c
> @@ -35,35 +35,40 @@ static int64_t
> icu_on_off_from_str(const char *str, uint32_t len)
> {
> return strnindex(coll_icu_on_off_strs + 1, str, len,
> - coll_icu_on_off_MAX - 1) + 1;
> + coll_icu_on_off_MAX - 1) +
> + 1;
32. Ugly.
> }
>
> static int64_t
> icu_alternate_handling_from_str(const char *str, uint32_t len)
> {
> return strnindex(coll_icu_alternate_handling_strs + 1, str, len,
> - coll_icu_alternate_handling_MAX - 1) + 1;
> + coll_icu_alternate_handling_MAX - 1) +
> + 1;
33. Super ugly.
> }
>
> static int64_t
> icu_case_first_from_str(const char *str, uint32_t len)
> {
> return strnindex(coll_icu_case_first_strs + 1, str, len,
> - coll_icu_case_first_MAX - 1) + 1;
> + coll_icu_case_first_MAX - 1) +
> + 1;
34. Extra ugly.
> }
>
> static int64_t
> icu_strength_from_str(const char *str, uint32_t len)
> {
> return strnindex(coll_icu_strength_strs + 1, str, len,
> - coll_icu_strength_MAX - 1) + 1;
> + coll_icu_strength_MAX - 1) +
> + 1;
35. Incredibly ugly.
> }
> diff --git a/src/box/constraint_id.c b/src/box/constraint_id.c
> index ba6ed859c..1047c7e4d 100644
> --- a/src/box/constraint_id.c
> +++ b/src/box/constraint_id.c
> @@ -35,10 +35,10 @@
> #include "diag.h"
>
> const char *constraint_type_strs[] = {
> - [CONSTRAINT_TYPE_PK] = "PRIMARY KEY",
> - [CONSTRAINT_TYPE_UNIQUE] = "UNIQUE",
> - [CONSTRAINT_TYPE_FK] = "FOREIGN KEY",
> - [CONSTRAINT_TYPE_CK] = "CHECK",
> + [CONSTRAINT_TYPE_PK] = "PRIMARY KEY",
> + [CONSTRAINT_TYPE_UNIQUE] = "UNIQUE",
> + [CONSTRAINT_TYPE_FK] = "FOREIGN KEY",
> + [CONSTRAINT_TYPE_CK] = "CHECK",
> };
36. Do I really need to find each such place, where the alignment was fine,
and point at it for you? You could just take a look at the patch by yourself,
before sending it.
> diff --git a/src/box/errcode.c b/src/box/errcode.c
> index c1cb59476..778054fea 100644
> --- a/src/box/errcode.c
> +++ b/src/box/errcode.c
> @@ -31,12 +31,7 @@
> */
> #include "errcode.h"
>
> -#define ERRCODE_RECORD_MEMBER(s, d) { \
> - .errstr = #s, \
> - .errdesc = d \
> -},
> -
> -struct errcode_record box_error_codes[box_error_code_MAX] = {
> - ERROR_CODES(ERRCODE_RECORD_MEMBER)
> -};
> +#define ERRCODE_RECORD_MEMBER(s, d) { .errstr = #s, .errdesc = d },
>
> +struct errcode_record box_error_codes[box_error_code_MAX] = { ERROR_CODES(
> + ERRCODE_RECORD_MEMBER) };
37. Previous version was more readable.
> diff --git a/src/box/error.h b/src/box/error.h
> index 338121dd9..7bab7e812 100644
> --- a/src/box/error.h
> +++ b/src/box/error.h
> @@ -189,43 +189,33 @@ extern const struct type_info type_CustomError;
> struct rmean;
> extern "C" struct rmean *rmean_error;
>
> -enum rmean_error_name {
> - RMEAN_ERROR,
> - RMEAN_ERROR_LAST
> -};
> +enum rmean_error_name { RMEAN_ERROR, RMEAN_ERROR_LAST };
38. We never write enum names in one line. It is harder to
read, harder to comment, and leads to unnecessary git history
rewrite each time when a new value is added.
> extern const char *rmean_error_strings[RMEAN_ERROR_LAST];
>
> -class ClientError: public Exception
> -{
> +class ClientError: public Exception {
> public:
> - virtual void raise()
> - {
> - throw this;
> - }
> + virtual void raise() { throw this; }
39. We do not put return type on the same line as the
function name. In new code. Also function body is always
on a separate line, with {} having their own lines.
>
> virtual void log() const;
>
> - int
> - errcode() const
> - {
> - return m_errcode;
> - }
> + int errcode() const { return m_errcode; }
>
> ClientError(const char *file, unsigned line, uint32_t errcode, ...);
>
> static uint32_t get_errcode(const struct error *e);
> /* client errno code */
> int m_errcode;
> diff --git a/src/box/execute.c b/src/box/execute.c
> index e14da20be..4c6ce98de 100644
> --- a/src/box/execute.c
> +++ b/src/box/execute.c
> @@ -305,7 +303,7 @@ metadata_map_sizeof(const char *name, const char *type, const char *coll,
> members_count++;
> map_size += mp_sizeof_uint(IPROTO_FIELD_SPAN);
> map_size += span != NULL ? mp_sizeof_str(strlen(span)) :
> - mp_sizeof_nil();
> + mp_sizeof_nil();
40. +21 char, this is something new.
> }
> map_size += mp_sizeof_uint(IPROTO_FIELD_NAME);
> map_size += mp_sizeof_uint(IPROTO_FIELD_TYPE);
> @@ -592,8 +591,8 @@ sql_reprepare(struct sql_stmt **stmt)
> {
> const char *sql_str = sql_stmt_query_str(*stmt);
> struct sql_stmt *new_stmt;
> - if (sql_stmt_compile(sql_str, strlen(sql_str), NULL,
> - &new_stmt, NULL) != 0)
> + if (sql_stmt_compile(sql_str, strlen(sql_str), NULL, &new_stmt, NULL) !=
> + 0)
41. Ugly.
> return -1;
> if (sql_stmt_cache_update(*stmt, new_stmt) != 0)
> return -1;
> @@ -631,7 +630,8 @@ sql_prepare(const char *sql, int len, struct port *port)
> if (!session_check_stmt_id(current_session(), stmt_id))
> session_add_stmt_id(current_session(), stmt_id);
> enum sql_serialization_format format = sql_column_count(stmt) > 0 ?
> - DQL_PREPARE : DML_PREPARE;
> + DQL_PREPARE :
> + DML_PREPARE;
42. Ugly.
> port_sql_create(port, stmt, format, false);
>
> return 0;
> @@ -677,8 +677,8 @@ sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)
> if (column_count > 0) {
> /* Either ROW or DONE or ERROR. */
> while ((rc = sql_step(stmt)) == SQL_ROW) {
> - if (sql_row_to_port(stmt, column_count, region,
> - port) != 0)
> + if (sql_row_to_port(stmt, column_count, region, port) !=
> + 0)
43. Ugly.
> return -1;
> }
> assert(rc == SQL_DONE || rc != 0);
> @@ -721,7 +720,8 @@ sql_execute_prepared(uint32_t stmt_id, const struct sql_bind *bind,
> if (sql_bind(stmt, bind, bind_count) != 0)
> return -1;
> enum sql_serialization_format format = sql_column_count(stmt) > 0 ?
> - DQL_EXECUTE : DML_EXECUTE;
> + DQL_EXECUTE :
> + DML_EXECUTE;
44. Ugly and random.
> port_sql_create(port, stmt, format, false);
> if (sql_execute(stmt, port, region) != 0) {
> port_destroy(port);
> @@ -743,7 +743,8 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
> return -1;
> assert(stmt != NULL);
> enum sql_serialization_format format = sql_column_count(stmt) > 0 ?
> - DQL_EXECUTE : DML_EXECUTE;
> + DQL_EXECUTE :
> + DML_EXECUTE;
45. Ugly and random.
> port_sql_create(port, stmt, format, true);
> if (sql_bind(stmt, bind, bind_count) == 0 &&
> sql_execute(stmt, port, region) == 0)
> diff --git a/src/box/field_def.c b/src/box/field_def.c
> index 34cecfa6d..4bccbb158 100644
> --- a/src/box/field_def.c
> +++ b/src/box/field_def.c
> @@ -165,15 +166,14 @@ const struct opt_def field_def_reg[] = {
> OPT_END,
> };
>
> -const struct field_def field_def_default = {
> - .type = FIELD_TYPE_ANY,
> - .name = NULL,
> - .is_nullable = false,
> - .nullable_action = ON_CONFLICT_ACTION_DEFAULT,
> - .coll_id = COLL_NONE,
> - .default_value = NULL,
> - .default_value_expr = NULL
> -};
> +const struct field_def field_def_default = { .type = FIELD_TYPE_ANY,
> + .name = NULL,
> + .is_nullable = false,
> + .nullable_action =
> + ON_CONFLICT_ACTION_DEFAULT,
> + .coll_id = COLL_NONE,
> + .default_value = NULL,
> + .default_value_expr = NULL };
46. Extremely ugly.
> diff --git a/src/box/func.c b/src/box/func.c
> index 8087c953f..f0c2b8870 100644
> --- a/src/box/func.c
> +++ b/src/box/func.c
> @@ -618,7 +621,8 @@ func_access_check(struct func *func)
> return 0;
> user_access_t access = PRIV_X | PRIV_U;
> /* Check access for all functions. */
> - access &= ~entity_access_get(SC_FUNCTION)[credentials->auth_token].effective;
> + access &= ~entity_access_get(SC_FUNCTION)[credentials->auth_token]
> + .effective;
47. Ugly.
> user_access_t func_access = access & ~credentials->universal_access;
> if ((func_access & PRIV_U) != 0 ||
> (func->def->uid != credentials->uid &&
> diff --git a/src/box/gc.c b/src/box/gc.c
> index 76f7c6325..2c8a9f851 100644
> --- a/src/box/gc.c
> +++ b/src/box/gc.c
> @@ -54,17 +54,15 @@
> #include "say.h"
> #include "vclock.h"
> #include "cbus.h"
> -#include "engine.h" /* engine_collect_garbage() */
> -#include "wal.h" /* wal_collect_garbage() */
> +#include "engine.h" /* engine_collect_garbage() */
> +#include "wal.h" /* wal_collect_garbage() */
48. Both comments are outdated. This file also
uses engine_begin_checkpoint and wal_begin_checkpoint, at least.
> #include "checkpoint_schedule.h"
> #include "txn_limbo.h"
>
> struct gc_state gc;
>
> -static int
> -gc_cleanup_fiber_f(va_list);
> -static int
> -gc_checkpoint_fiber_f(va_list);
> +static int gc_cleanup_fiber_f(va_list);
> +static int gc_checkpoint_fiber_f(va_list);
49. We always declare return type on a separate line.
>
> /**
> diff --git a/src/box/index.h b/src/box/index.h
> index 6225a8674..bf0f2bb3e 100644
> --- a/src/box/index.h
> +++ b/src/box/index.h
> @@ -476,8 +476,8 @@ replace_check_dup(struct tuple *old_tuple, struct tuple *dup_tuple,
> * dup_replace_mode is DUP_REPLACE, and
> * a tuple with the same key is not found.
> */
> - return old_tuple ?
> - ER_CANT_UPDATE_PRIMARY_KEY : ER_TUPLE_NOT_FOUND;
> + return old_tuple ? ER_CANT_UPDATE_PRIMARY_KEY :
> + ER_TUPLE_NOT_FOUND;
50. Ugly and random.
> }
> } else { /* dup_tuple != NULL */
> diff --git a/src/box/index_def.h b/src/box/index_def.h
> index 2180a69cf..1ed3dc14f 100644
> --- a/src/box/index_def.h
> +++ b/src/box/index_def.h
> @@ -207,8 +207,8 @@ index_opts_cmp(const struct index_opts *o1, const struct index_opts *o2)
> if (o1->page_size != o2->page_size)
> return o1->page_size < o2->page_size ? -1 : 1;
> if (o1->run_count_per_level != o2->run_count_per_level)
> - return o1->run_count_per_level < o2->run_count_per_level ?
> - -1 : 1;
> + return o1->run_count_per_level < o2->run_count_per_level ? -1 :
> + 1;
51. Well, what the fuck :) ?
> if (o1->run_size_ratio != o2->run_size_ratio)
> return o1->run_size_ratio < o2->run_size_ratio ? -1 : 1;
> if (o1->bloom_fpr != o2->bloom_fpr)
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index b8f65e5ec..950bde25e 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -56,7 +56,7 @@
> #include "tuple_convert.h"
> #include "session.h"
> #include "xrow.h"
> -#include "schema.h" /* schema_version */
> +#include "schema.h" /* schema_version */
52. What is the logic? In some places it forces to use 1 space
between code and comment. In others it adds more. It is
inconsistent.
> #include "replication.h" /* instance_uuid */
> #include "iproto_constants.h"
> #include "rmean.h"
> @@ -339,11 +338,8 @@ iproto_process_push(struct cmsg *m);
> static void
> tx_end_push(struct cmsg *m);
>
> -static const struct cmsg_hop push_route[] = {
> - { iproto_process_push, &tx_pipe },
> - { tx_end_push, NULL }
> -};
> -
> +static const struct cmsg_hop push_route[] = { { iproto_process_push, &tx_pipe },
> + { tx_end_push, NULL } };
>
53. Ugly.
> /* }}} */
> @@ -522,8 +517,8 @@ struct iproto_connection
> */
> struct {
> alignas(CACHELINE_SIZE)
> - /** Pointer to the current output buffer. */
> - struct obuf *p_obuf;
> + /** Pointer to the current output buffer. */
> + struct obuf *p_obuf;
54. Wat?
> /** True if Kharon is in use/travelling. */
> bool is_push_sent;
> /**
> @@ -821,19 +816,18 @@ iproto_enqueue_batch(struct iproto_connection *con, struct ibuf *in)
> /* Read request length. */
> if (mp_typeof(*pos) != MP_UINT) {
> errmsg = "packet length";
> -err_msgpack:
> + err_msgpack:
55. Jump to a label can be done even out of the {} block,
where it is declared. So this change is simply incorrect.
> cpipe_flush_input(&tx_pipe);
> - diag_set(ClientError, ER_INVALID_MSGPACK,
> - errmsg);
> + diag_set(ClientError, ER_INVALID_MSGPACK, errmsg);
> return -1;
> }
> if (mp_check_uint(pos, in->wpos) >= 0)
> break;
> @@ -1054,11 +1047,12 @@ iproto_flush(struct iproto_connection *con)
> size_t offset = 0;
> int advance = 0;
> advance = sio_move_iov(iov, nwr, &offset);
> - begin->used += nwr; /* advance write position */
> - begin->iov_len = advance == 0 ? begin->iov_len + offset: offset;
> + begin->used += nwr; /* advance write position */
> + begin->iov_len = advance == 0 ? begin->iov_len + offset :
> + offset;
56. Last line ugly and random.
> begin->pos += advance;
> assert(begin->pos <= end->pos);
> - } else if (nwr < 0 && ! sio_wouldblock(errno)) {
> + } else if (nwr < 0 && !sio_wouldblock(errno)) {
> diag_raise();
> }
> @@ -1213,20 +1206,20 @@ static const struct cmsg_hop sql_route[] = {
> };
>
> static const struct cmsg_hop *dml_route[IPROTO_TYPE_STAT_MAX] = {
> - NULL, /* IPROTO_OK */
> - select_route, /* IPROTO_SELECT */
> - process1_route, /* IPROTO_INSERT */
> - process1_route, /* IPROTO_REPLACE */
> - process1_route, /* IPROTO_UPDATE */
> - process1_route, /* IPROTO_DELETE */
> - call_route, /* IPROTO_CALL_16 */
> - misc_route, /* IPROTO_AUTH */
> - call_route, /* IPROTO_EVAL */
> - process1_route, /* IPROTO_UPSERT */
> - call_route, /* IPROTO_CALL */
> - sql_route, /* IPROTO_EXECUTE */
> - NULL, /* IPROTO_NOP */
> - sql_route, /* IPROTO_PREPARE */
> + NULL, /* IPROTO_OK */
> + select_route, /* IPROTO_SELECT */
> + process1_route, /* IPROTO_INSERT */
> + process1_route, /* IPROTO_REPLACE */
> + process1_route, /* IPROTO_UPDATE */
> + process1_route, /* IPROTO_DELETE */
> + call_route, /* IPROTO_CALL_16 */
> + misc_route, /* IPROTO_AUTH */
> + call_route, /* IPROTO_EVAL */
> + process1_route, /* IPROTO_UPSERT */
> + call_route, /* IPROTO_CALL */
> + sql_route, /* IPROTO_EXECUTE */
> + NULL, /* IPROTO_NOP */
> + sql_route, /* IPROTO_PREPARE */
57. I already said it for a few places, and will repeat here - the alignment
was correct beforehand. Now addition of any function with name longer than
process1_route will change this whole array, making the diff bigger and
ruining the history.
> };
> @@ -2051,24 +2041,21 @@ static struct evio_service binary; /* iproto binary listener */
> * The network io thread main function:
> * begin serving the message bus.
> */
> -static int
> -net_cord_f(va_list /* ap */)
> +static int net_cord_f(va_list /* ap */)
58. This change is incorrect. Return type should be on separate line.
> {
> mempool_create(&iproto_msg_pool, &cord()->slabc,
> sizeof(struct iproto_msg));
> mempool_create(&iproto_connection_pool, &cord()->slabc,
> sizeof(struct iproto_connection));
>
> - evio_service_init(loop(), &binary, "binary",
> - iproto_on_accept, NULL);
> -
> + evio_service_init(loop(), &binary, "binary", iproto_on_accept, NULL);
>
> /* Init statistics counter */
> rmean_net = rmean_new(rmean_net_strings, IPROTO_LAST);
>
> if (rmean_net == NULL) {
> - tnt_raise(OutOfMemory, sizeof(struct rmean),
> - "rmean", "struct rmean");
> + tnt_raise(OutOfMemory, sizeof(struct rmean), "rmean",
> + "struct rmean");
> }
> @@ -2204,10 +2191,7 @@ iproto_init(void)
> }
>
> /** Available iproto configuration changes. */
> -enum iproto_cfg_op {
> - IPROTO_CFG_MSG_MAX,
> - IPROTO_CFG_LISTEN
> -};
> +enum iproto_cfg_op { IPROTO_CFG_MSG_MAX, IPROTO_CFG_LISTEN };
59. Enum values should be on separate lines.
> diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
> index d3738c705..eb0926bbe 100644
> --- a/src/box/iproto_constants.h
> +++ b/src/box/iproto_constants.h
> @@ -94,7 +94,7 @@ enum iproto_key {
>
> /* Also request keys. See the comment above. */
> IPROTO_EXPR = 0x27, /* EVAL */
> - IPROTO_OPS = 0x28, /* UPSERT but not UPDATE ops, because of legacy */
> + IPROTO_OPS = 0x28, /* UPSERT but not UPDATE ops, because of legacy */
59. Why?
> IPROTO_BALLOT = 0x29,
> IPROTO_TUPLE_META = 0x2a,
> IPROTO_OPTIONS = 0x2b,
> diff --git a/src/box/iterator_type.h b/src/box/iterator_type.h
> index c57e61407..5a66701d5 100644
> --- a/src/box/iterator_type.h
> +++ b/src/box/iterator_type.h
> @@ -61,18 +61,19 @@ extern "C" {
> */
> enum iterator_type {
> /* ITER_EQ must be the first member for request_create */
> - ITER_EQ = 0, /* key == x ASC order */
> - ITER_REQ = 1, /* key == x DESC order */
> - ITER_ALL = 2, /* all tuples */
> - ITER_LT = 3, /* key < x */
> - ITER_LE = 4, /* key <= x */
> - ITER_GE = 5, /* key >= x */
> - ITER_GT = 6, /* key > x */
> - ITER_BITS_ALL_SET = 7, /* all bits from x are set in key */
> - ITER_BITS_ANY_SET = 8, /* at least one x's bit is set */
> - ITER_BITS_ALL_NOT_SET = 9, /* all bits are not set */
> - ITER_OVERLAPS = 10, /* key overlaps x */
> - ITER_NEIGHBOR = 11, /* tuples in distance ascending order from specified point */
> + ITER_EQ = 0, /* key == x ASC order */
> + ITER_REQ = 1, /* key == x DESC order */
> + ITER_ALL = 2, /* all tuples */
> + ITER_LT = 3, /* key < x */
> + ITER_LE = 4, /* key <= x */
> + ITER_GE = 5, /* key >= x */
> + ITER_GT = 6, /* key > x */
> + ITER_BITS_ALL_SET = 7, /* all bits from x are set in key */
> + ITER_BITS_ANY_SET = 8, /* at least one x's bit is set */
> + ITER_BITS_ALL_NOT_SET = 9, /* all bits are not set */
> + ITER_OVERLAPS = 10, /* key overlaps x */
> + ITER_NEIGHBOR =
> + 11, /* tuples in distance ascending order from specified point */
> iterator_type_MAX
60. This whole hunk is dubious, but the last line is especially
ugly.
> diff --git a/src/box/key_def.c b/src/box/key_def.c
> index 93bb5515b..356bc4f8a 100644
> --- a/src/box/key_def.c
> +++ b/src/box/key_def.c
> @@ -43,15 +43,13 @@
>
> const char *sort_order_strs[] = { "asc", "desc", "undef" };
>
> -const struct key_part_def key_part_def_default = {
> - 0,
> - field_type_MAX,
> - COLL_NONE,
> - false,
> - ON_CONFLICT_ACTION_DEFAULT,
> - SORT_ORDER_ASC,
> - NULL
> -};
> +const struct key_part_def key_part_def_default = { 0,
> + field_type_MAX,
> + COLL_NONE,
> + false,
> + ON_CONFLICT_ACTION_DEFAULT,
> + SORT_ORDER_ASC,
> + NULL };
61. Super ugly.
> @@ -872,8 +874,8 @@ key_def_decode_parts_166(struct key_part_def *parts, uint32_t part_count,
> return -1;
> }
> part->is_nullable = (part->fieldno < field_count ?
> - fields[part->fieldno].is_nullable :
> - key_part_def_default.is_nullable);
> + fields[part->fieldno].is_nullable :
> + key_part_def_default.is_nullable);
62. Ugly and random.
> part->coll_id = COLL_NONE;
> part->path = NULL;
> }
> diff --git a/src/box/key_def.h b/src/box/key_def.h
> index 12abd1f09..a9a697376 100644
> --- a/src/box/key_def.h
> +++ b/src/box/key_def.h
> @@ -834,10 +823,11 @@ key_def_incomparable_type(const struct key_def *key_def)
> * @retval -1 mp_type is invalid.
> */
> static inline int
> -key_part_validate(enum field_type key_type, const char *key,
> - uint32_t field_no, bool is_nullable)
> +key_part_validate(enum field_type key_type, const char *key, uint32_t field_no,
> + bool is_nullable)
> {
> - if (unlikely(!field_mp_type_is_compatible(key_type, key, is_nullable))) {
> + if (unlikely(
> + !field_mp_type_is_compatible(key_type, key, is_nullable))) {
63. Ugly.
> diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
> field_type_strs[key_type]);
> return -1;
> diff --git a/src/box/key_list.c b/src/box/key_list.c
> index 6143b84a1..a604a1dce 100644
> --- a/src/box/key_list.c
> +++ b/src/box/key_list.c
> @@ -159,12 +159,12 @@ key_list_iterator_next(struct key_list_iterator *it, const char **value)
> diag_set(ClientError, ER_FUNC_INDEX_FORMAT, it->index_def->name,
> space ? space_name(space) : "",
> tt_sprintf(tnt_errcode_desc(ER_EXACT_MATCH),
> - key_def->part_count, part_count));
> + key_def->part_count, part_count));
> return -1;
> }
> const char *key_end;
> - if (key_validate_parts(key_def, rptr, part_count, true,
> - &key_end) != 0) {
> + if (key_validate_parts(key_def, rptr, part_count, true, &key_end) !=
> + 0) {
64. Ugly.
> struct space *space = space_by_id(it->index_def->space_id);
> /*
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index 0315e720c..a569e9c9e 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -403,8 +401,8 @@ struct encode_lua_ctx {
> static int
> encode_lua_call(lua_State *L)
> {
> - struct encode_lua_ctx *ctx =
> - (struct encode_lua_ctx *) lua_topointer(L, 1);
> + struct encode_lua_ctx *ctx = (struct encode_lua_ctx *)lua_topointer(L,
> + 1);
65. Ugly.
> /*
> * Add all elements from Lua stack to the buffer.
> *
> @@ -424,8 +422,8 @@ encode_lua_call(lua_State *L)
> static int
> encode_lua_call_16(lua_State *L)
> {
> - struct encode_lua_ctx *ctx =
> - (struct encode_lua_ctx *) lua_topointer(L, 1);
> + struct encode_lua_ctx *ctx = (struct encode_lua_ctx *)lua_topointer(L,
> + 1);
66. Ugly.
> /*
> * Add all elements from Lua stack to the buffer.
> *
> @@ -1022,15 +1020,15 @@ lbox_func_new_or_delete(struct trigger *trigger, void *event)
> return 0;
> }
>
> -static struct trigger on_alter_func_in_lua = {
> - RLIST_LINK_INITIALIZER, lbox_func_new_or_delete, NULL, NULL
> -};
> +static struct trigger on_alter_func_in_lua = { RLIST_LINK_INITIALIZER,
> + lbox_func_new_or_delete, NULL,
> + NULL };
67. Ugly.
> static const struct luaL_Reg boxlib_internal[] = {
> - {"call_loadproc", lbox_call_loadproc},
> - {"module_reload", lbox_module_reload},
> - {"func_call", lbox_func_call},
> - {NULL, NULL}
> + { "call_loadproc", lbox_call_loadproc },
> + { "module_reload", lbox_module_reload },
> + { "func_call", lbox_func_call },
> + { NULL, NULL }
> };
> diff --git a/src/box/lua/console.c b/src/box/lua/console.c
> index ea5385c74..b2331607c 100644
> --- a/src/box/lua/console.c
> +++ b/src/box/lua/console.c
> @@ -281,8 +280,8 @@ lbox_console_readline(struct lua_State *L)
> rl_callback_handler_install(prompt, console_push_line);
> top = lua_gettop(L);
> while (top == lua_gettop(L)) {
> - while (coio_wait(STDIN_FILENO, COIO_READ,
> - TIMEOUT_INFINITY) == 0) {
> + while (coio_wait(STDIN_FILENO, COIO_READ, TIMEOUT_INFINITY) ==
> + 0) {
68. Ugly.
> /*
> * Make sure the user of interactive
> * console has not hanged us, otherwise
> @@ -757,20 +759,23 @@ lua_rl_dmadd(dmlist *ml, const char *p, size_t pn, const char *s, int suf)
>
> if (s) {
> size_t n = strlen(s);
> - if (!(t = (char *)malloc(sizeof(char)*(pn + n + 2))))
> + if (!(t = (char *)malloc(sizeof(char) * (pn + n + 2))))
> return 1;
> memcpy(t, p, pn);
> memcpy(t + pn, s, n);
> n += pn;
> t[n] = suf;
> - if (suf) t[++n] = '\0';
> + if (suf)
> + t[++n] = '\0';
>
> if (ml->idx == 0) {
> ml->matchlen = n;
> } else {
> size_t i;
> for (i = 0; i < ml->matchlen && i < n &&
> - ml->list[1][i] == t[i]; i++) ;
> + ml->list[1][i] == t[i];
> + i++)
> + ;
69. Ugly.
> /* Set matchlen to common prefix. */
> ml->matchlen = i;
> }
> @@ -929,7 +932,7 @@ lua_rl_complete(lua_State *L, const char *text, int start, int end)
> lua_pop(L, 1);
>
> if (ml.idx == 0) {
> -error:
> + error:
70. Labels are visible everywhere inside a function, and should not
have indentation.
> lua_rl_dmfree(&ml);
> lua_settop(L, savetop);
> return NULL;
> diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
> index 54ec28468..3a51d93db 100644
> --- a/src/box/lua/error.cc
> +++ b/src/box/lua/error.cc
> @@ -334,12 +334,11 @@ box_lua_error_init(struct lua_State *L) {
>
> lua_pop(L, 1);
>
> - static const struct luaL_Reg errinjlib[] = {
> - {"info", lbox_errinj_info},
> - {"set", lbox_errinj_set},
> - {"get", lbox_errinj_get},
> - {NULL, NULL}
> - };
> + static const struct luaL_Reg errinjlib[] = { { "info",
> + lbox_errinj_info },
> + { "set", lbox_errinj_set },
> + { "get", lbox_errinj_get },
> + { NULL, NULL } };
71. Ugly.
> /* box.error.injection is not set by register_module */
> luaL_register_module(L, "box.error.injection", errinjlib);
> lua_pop(L, 1);
> diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
> index 926a0a61c..a5e1a3cb9 100644
> --- a/src/box/lua/execute.c
> +++ b/src/box/lua/execute.c
> @@ -192,7 +192,8 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
> lua_newtable(L);
> int i = 1;
> struct autoinc_id_entry *id_entry;
> - stailq_foreach_entry(id_entry, autoinc_id_list, link) {
> + stailq_foreach_entry(id_entry, autoinc_id_list, link)
> + {
72. This is a for loop, and should be formatted like a for loop.
> if (id_entry->id >= 0)
> luaL_pushuint64(L, id_entry->id);
> else
> diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> index cac3fd475..29dfca200 100644
> --- a/src/box/lua/info.c
> +++ b/src/box/lua/info.c
> @@ -60,7 +60,8 @@ lbox_pushvclock(struct lua_State *L, const struct vclock *vclock)
> lua_createtable(L, 0, vclock_size(vclock));
> struct vclock_iterator it;
> vclock_iterator_init(&it, vclock);
> - vclock_foreach(&it, replica) {
> + vclock_foreach(&it, replica)
> + {
73. Ditto.
> lua_pushinteger(L, replica.id);
> luaL_pushuint64(L, replica.lsn);
> lua_settable(L, -3);
> @@ -91,7 +92,8 @@ lbox_pushapplier(lua_State *L, struct applier *applier)
> char *d = status;
> const char *s = applier_state_strs[applier->state] + strlen("APPLIER_");
> assert(strlen(s) < sizeof(status));
> - while ((*(d++) = tolower(*(s++))));
> + while ((*(d++) = tolower(*(s++))))
> + ;
74. Ugly.
>
> lua_pushstring(L, "status");
> lua_pushstring(L, status);
> @@ -202,7 +205,8 @@ lbox_info_replication(struct lua_State *L)
> lua_setfield(L, -2, "__serialize");
> lua_setmetatable(L, -2);
>
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
75. This is a for loop, and should be formatted like a for loop.
> /* Applier hasn't received replica id yet */
> if (replica->id == REPLICA_ID_NIL)
> continue;
> @@ -226,7 +230,8 @@ lbox_info_replication_anon_call(struct lua_State *L)
> lua_setfield(L, -2, "__serialize");
> lua_setmetatable(L, -2);
>
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
76. Ditto.
> if (!replica->anon)
> continue;
>
> @@ -450,7 +455,8 @@ lbox_info_gc_call(struct lua_State *L)
>
> count = 0;
> struct gc_checkpoint *checkpoint;
> - gc_foreach_checkpoint(checkpoint) {
> + gc_foreach_checkpoint(checkpoint)
> + {
77. Ditto.
> lua_createtable(L, 0, 2);
>
> lua_pushstring(L, "vclock");
> @@ -465,7 +471,8 @@ lbox_info_gc_call(struct lua_State *L)
> lua_newtable(L);
> int ref_idx = 0;
> struct gc_checkpoint_ref *ref;
> - gc_foreach_checkpoint_ref(ref, checkpoint) {
> + gc_foreach_checkpoint_ref(ref, checkpoint)
> + {
78. Ditto.
> lua_pushstring(L, ref->name);
> lua_rawseti(L, -2, ++ref_idx);
> }
> diff --git a/src/box/lua/init.c b/src/box/lua/init.c
> index d0316ef86..6a89516fb 100644
> --- a/src/box/lua/init.c
> +++ b/src/box/lua/init.c
> @@ -70,41 +70,28 @@
>
> static uint32_t CTID_STRUCT_TXN_SAVEPOINT_PTR = 0;
>
> -extern char session_lua[],
> - tuple_lua[],
> - key_def_lua[],
> - schema_lua[],
> - load_cfg_lua[],
> - xlog_lua[],
> +extern char session_lua[], tuple_lua[], key_def_lua[], schema_lua[],
> + load_cfg_lua[], xlog_lua[],
> #if ENABLE_FEEDBACK_DAEMON
> feedback_daemon_lua[],
> #endif
> - net_box_lua[],
> - upgrade_lua[],
> - console_lua[],
> - merger_lua[];
> -
> -static const char *lua_sources[] = {
> - "box/session", session_lua,
> - "box/tuple", tuple_lua,
> - "box/schema", schema_lua,
> + net_box_lua[], upgrade_lua[], console_lua[], merger_lua[];
> +
> +static const char *lua_sources[] = { "box/session", session_lua, "box/tuple",
> + tuple_lua, "box/schema", schema_lua,
> #if ENABLE_FEEDBACK_DAEMON
> - /*
> + /*
79. Wtf happened to the comment's first line?
> * It is important to initialize the daemon before
> * load_cfg, because the latter picks up some values
> * from the feedback daemon.
> */
> - "box/feedback_daemon", feedback_daemon_lua,
> + "box/feedback_daemon", feedback_daemon_lua,
> #endif
> - "box/upgrade", upgrade_lua,
> - "box/net_box", net_box_lua,
> - "box/console", console_lua,
> - "box/load_cfg", load_cfg_lua,
> - "box/xlog", xlog_lua,
> - "box/key_def", key_def_lua,
> - "box/merger", merger_lua,
> - NULL
> -};
> + "box/upgrade", upgrade_lua, "box/net_box",
> + net_box_lua, "box/console", console_lua,
> + "box/load_cfg", load_cfg_lua, "box/xlog",
> + xlog_lua, "box/key_def", key_def_lua,
> + "box/merger", merger_lua, NULL };
>
80. This is one of the ugliest things I saw in this patch.
This whole hunk. Did you actually look at it? I mean in the file.
Not at the diff. It looks like some automatically generated code
not intended to be read by a human.
> static int
> lbox_commit(lua_State *L)
> @@ -313,18 +300,20 @@ lbox_push_txn(struct lua_State *L, void *event)
> * @sa lbox_trigger_reset.
> */
> #define LBOX_TXN_TRIGGER(name) \
> -static int \
> -lbox_on_##name(struct lua_State *L) { \
> - struct txn *txn = in_txn(); \
> - int top = lua_gettop(L); \
> - if (top > 2 || txn == NULL) { \
> - return luaL_error(L, "Usage inside a transaction: " \
> - "box.on_" #name "([function | nil, " \
> - "[function | nil]])"); \
> - } \
> - txn_init_triggers(txn); \
> - return lbox_trigger_reset(L, 2, &txn->on_##name, lbox_push_txn, NULL); \
> -}
> + static int lbox_on_##name(struct lua_State *L) \
81. The formatting is wrong. Function names should be on separate
line from return type. Also we don't use +1 tab inside macros.
> + { \
> + struct txn *txn = in_txn(); \
> + int top = lua_gettop(L); \
> + if (top > 2 || txn == NULL) { \
> + return luaL_error(L, \
> + "Usage inside a transaction: " \
> + "box.on_" #name "([function | nil, " \
> + "[function | nil]])"); \
> + } \
> + txn_init_triggers(txn); \
> + return lbox_trigger_reset(L, 2, &txn->on_##name, \
> + lbox_push_txn, NULL); \
> + }
>
> LBOX_TXN_TRIGGER(commit)
> LBOX_TXN_TRIGGER(rollback)
> @@ -384,21 +373,18 @@ lbox_backup_stop(struct lua_State *L)
> return 0;
> }
>
> -static const struct luaL_Reg boxlib[] = {
> - {"commit", lbox_commit},
> - {"rollback", lbox_rollback},
> - {"on_commit", lbox_on_commit},
> - {"on_rollback", lbox_on_rollback},
> - {"snapshot", lbox_snapshot},
> - {"rollback_to_savepoint", lbox_rollback_to_savepoint},
> - {NULL, NULL}
> -};
> +static const struct luaL_Reg boxlib[] = { { "commit", lbox_commit },
> + { "rollback", lbox_rollback },
> + { "on_commit", lbox_on_commit },
> + { "on_rollback", lbox_on_rollback },
> + { "snapshot", lbox_snapshot },
> + { "rollback_to_savepoint",
> + lbox_rollback_to_savepoint },
> + { NULL, NULL } };
>
> -static const struct luaL_Reg boxlib_backup[] = {
> - {"start", lbox_backup_start},
> - {"stop", lbox_backup_stop},
> - {NULL, NULL}
> -};
> +static const struct luaL_Reg boxlib_backup[] = { { "start", lbox_backup_start },
> + { "stop", lbox_backup_stop },
> + { NULL, NULL } };
82. Unreadable.
>
> /**
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 0b6c362ae..d2aa3efb9 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -494,20 +493,18 @@ netbox_communicate(lua_State *L)
> int revents = COIO_READ;
> while (true) {
> /* reader serviced first */
> -check_limit:
> + check_limit:
83. Labels are visible everywhere inside a function, and should not have
indentation.
> if (ibuf_used(recv_buf) >= limit) {
> lua_pushnil(L);
> lua_pushinteger(L, (lua_Integer)limit);
> return 2;
> }
> diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c
> index caa08a60f..32dcf478b 100644
> --- a/src/box/lua/serialize_lua.c
> +++ b/src/box/lua/serialize_lua.c
> @@ -45,36 +45,36 @@
> #include "serialize_lua.h"
>
> #if 0
> -# define SERIALIZER_TRACE
> +#define SERIALIZER_TRACE
> #endif
>
> /* Serializer for Lua output mode */
> static struct luaL_serializer *serializer_lua;
>
> enum {
> - NODE_NONE_BIT = 0,
> - NODE_ROOT_BIT = 1,
> - NODE_RAW_BIT = 2,
> - NODE_LVALUE_BIT = 3,
> - NODE_RVALUE_BIT = 4,
> - NODE_MAP_KEY_BIT = 5,
> - NODE_MAP_VALUE_BIT = 6,
> - NODE_EMBRACE_BIT = 7,
> - NODE_QUOTE_BIT = 8,
> + NODE_NONE_BIT = 0,
> + NODE_ROOT_BIT = 1,
> + NODE_RAW_BIT = 2,
> + NODE_LVALUE_BIT = 3,
> + NODE_RVALUE_BIT = 4,
> + NODE_MAP_KEY_BIT = 5,
> + NODE_MAP_VALUE_BIT = 6,
> + NODE_EMBRACE_BIT = 7,
> + NODE_QUOTE_BIT = 8,
>
> NODE_MAX
> };
>
> enum {
> - NODE_NONE = (1u << NODE_NONE_BIT),
> - NODE_ROOT = (1u << NODE_ROOT_BIT),
> - NODE_RAW = (1u << NODE_RAW_BIT),
> - NODE_LVALUE = (1u << NODE_LVALUE_BIT),
> - NODE_RVALUE = (1u << NODE_RVALUE_BIT),
> - NODE_MAP_KEY = (1u << NODE_MAP_KEY_BIT),
> - NODE_MAP_VALUE = (1u << NODE_MAP_VALUE_BIT),
> - NODE_EMBRACE = (1u << NODE_EMBRACE_BIT),
> - NODE_QUOTE = (1u << NODE_QUOTE_BIT),
> + NODE_NONE = (1u << NODE_NONE_BIT),
> + NODE_ROOT = (1u << NODE_ROOT_BIT),
> + NODE_RAW = (1u << NODE_RAW_BIT),
> + NODE_LVALUE = (1u << NODE_LVALUE_BIT),
> + NODE_RVALUE = (1u << NODE_RVALUE_BIT),
> + NODE_MAP_KEY = (1u << NODE_MAP_KEY_BIT),
> + NODE_MAP_VALUE = (1u << NODE_MAP_VALUE_BIT),
> + NODE_EMBRACE = (1u << NODE_EMBRACE_BIT),
> + NODE_QUOTE = (1u << NODE_QUOTE_BIT),
> };
>
> struct node {
> @@ -136,18 +136,13 @@ struct lua_dumper {
>
> #ifdef SERIALIZER_TRACE
>
> -#define __gen_mp_name(__v) [__v] = # __v
> +#define __gen_mp_name(__v) [__v] = #__v
> static const char *mp_type_names[] = {
> - __gen_mp_name(MP_NIL),
> - __gen_mp_name(MP_UINT),
> - __gen_mp_name(MP_INT),
> - __gen_mp_name(MP_STR),
> - __gen_mp_name(MP_BIN),
> - __gen_mp_name(MP_ARRAY),
> - __gen_mp_name(MP_MAP),
> - __gen_mp_name(MP_BOOL),
> - __gen_mp_name(MP_FLOAT),
> - __gen_mp_name(MP_DOUBLE),
> + __gen_mp_name(MP_NIL), __gen_mp_name(MP_UINT),
> + __gen_mp_name(MP_INT), __gen_mp_name(MP_STR),
> + __gen_mp_name(MP_BIN), __gen_mp_name(MP_ARRAY),
> + __gen_mp_name(MP_MAP), __gen_mp_name(MP_BOOL),
> + __gen_mp_name(MP_FLOAT), __gen_mp_name(MP_DOUBLE),
> __gen_mp_name(MP_EXT),
> };
>
> @@ -158,16 +153,12 @@ static const char *mp_ext_type_names[] = {
> };
> #undef __gen_mp_name
>
> -#define __gen_nd_name(__v) [__v ##_BIT] = # __v
> +#define __gen_nd_name(__v) [__v##_BIT] = #__v
> static const char *nd_type_names[] = {
> - __gen_nd_name(NODE_NONE),
> - __gen_nd_name(NODE_ROOT),
> - __gen_nd_name(NODE_RAW),
> - __gen_nd_name(NODE_LVALUE),
> - __gen_nd_name(NODE_RVALUE),
> - __gen_nd_name(NODE_MAP_KEY),
> - __gen_nd_name(NODE_MAP_VALUE),
> - __gen_nd_name(NODE_EMBRACE),
> + __gen_nd_name(NODE_NONE), __gen_nd_name(NODE_ROOT),
> + __gen_nd_name(NODE_RAW), __gen_nd_name(NODE_LVALUE),
> + __gen_nd_name(NODE_RVALUE), __gen_nd_name(NODE_MAP_KEY),
> + __gen_nd_name(NODE_MAP_VALUE), __gen_nd_name(NODE_EMBRACE),
> __gen_nd_name(NODE_QUOTE),
> };
84. All hunks in this file above this comment were better before.
Easier to read, easier to extend.
> #undef __gen_nd_name
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index 1ea993ccb..8b05a1aa2 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -117,16 +117,17 @@ lbox_space_on_replace(struct lua_State *L)
> int top = lua_gettop(L);
>
> if (top < 1 || !lua_istable(L, 1)) {
> - luaL_error(L,
> - "usage: space:on_replace(function | nil, [function | nil])");
> + luaL_error(
> + L,
> + "usage: space:on_replace(function | nil, [function | nil])");
85. Out of 80 symbols.
> }
> lua_getfield(L, 1, "id"); /* Get space id. */
> uint32_t id = lua_tonumber(L, lua_gettop(L));
> struct space *space = space_cache_find_xc(id);
> lua_pop(L, 1);
>
> - return lbox_trigger_reset(L, 3, &space->on_replace,
> - lbox_push_txn_stmt, NULL);
> + return lbox_trigger_reset(L, 3, &space->on_replace, lbox_push_txn_stmt,
> + NULL);
> }
>
> /**
> @@ -138,8 +139,9 @@ lbox_space_before_replace(struct lua_State *L)
> int top = lua_gettop(L);
>
> if (top < 1 || !lua_istable(L, 1)) {
> - luaL_error(L,
> - "usage: space:before_replace(function | nil, [function | nil])");
> + luaL_error(
> + L,
> + "usage: space:before_replace(function | nil, [function | nil])");
86. Out of 80 symbols.
> }
> lua_getfield(L, 1, "id"); /* Get space id. */
> @@ -529,9 +530,9 @@ box_lua_space_new_or_delete(struct trigger *trigger, void *event)
> return 0;
> }
>
> -static struct trigger on_alter_space_in_lua = {
> - RLIST_LINK_INITIALIZER, box_lua_space_new_or_delete, NULL, NULL
> -};
> +static struct trigger on_alter_space_in_lua = { RLIST_LINK_INITIALIZER,
> + box_lua_space_new_or_delete,
> + NULL, NULL };
>
87. Ugly.
> /**
> * Make a tuple or a table Lua object by map.
> @@ -708,8 +710,7 @@ box_lua_space_init(struct lua_State *L)
> lua_pop(L, 2); /* box, schema */
>
> static const struct luaL_Reg space_internal_lib[] = {
> - {"frommap", lbox_space_frommap},
> - {NULL, NULL}
> + { "frommap", lbox_space_frommap }, { NULL, NULL }
88. Ugly.
> };
> luaL_register(L, "box.internal.space", space_internal_lib);
> lua_pop(L, 1);
> diff --git a/src/box/lua/stat.c b/src/box/lua/stat.c
> index 29ec38b26..81dacdd68 100644
> --- a/src/box/lua/stat.c
> +++ b/src/box/lua/stat.c
> @@ -211,28 +211,25 @@ lbox_stat_sql(struct lua_State *L)
> return 1;
> }
>
> -static const struct luaL_Reg lbox_stat_meta [] = {
> - {"__index", lbox_stat_index},
> - {"__call", lbox_stat_call},
> - {NULL, NULL}
> -};
> +static const struct luaL_Reg lbox_stat_meta[] = { { "__index",
> + lbox_stat_index },
> + { "__call", lbox_stat_call },
> + { NULL, NULL } };
89. Ugly.
>
> -static const struct luaL_Reg lbox_stat_net_meta [] = {
> - {"__index", lbox_stat_net_index},
> - {"__call", lbox_stat_net_call},
> - {NULL, NULL}
> +static const struct luaL_Reg lbox_stat_net_meta[] = {
> + { "__index", lbox_stat_net_index },
> + { "__call", lbox_stat_net_call },
> + { NULL, NULL }
> };
>
> /** Initialize box.stat package. */
> void
> box_lua_stat_init(struct lua_State *L)
> {
> - static const struct luaL_Reg statlib [] = {
> - {"vinyl", lbox_stat_vinyl},
> - {"reset", lbox_stat_reset},
> - {"sql", lbox_stat_sql},
> - {NULL, NULL}
> - };
> + static const struct luaL_Reg statlib[] = { { "vinyl", lbox_stat_vinyl },
> + { "reset", lbox_stat_reset },
> + { "sql", lbox_stat_sql },
> + { NULL, NULL } };
90. Ugly.
>
> luaL_register_module(L, "box.stat", statlib);
> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> index 3e6f043b4..7343c78fb 100644
> --- a/src/box/lua/tuple.c
> +++ b/src/box/lua/tuple.c
> @@ -677,23 +680,19 @@ luaT_pushtuple(struct lua_State *L, box_tuple_t *tuple)
> }
>
> static const struct luaL_Reg lbox_tuple_meta[] = {
> - {"__gc", lbox_tuple_gc},
> - {"tostring", lbox_tuple_to_string},
> - {"slice", lbox_tuple_slice},
> - {"transform", lbox_tuple_transform},
> - {"tuple_to_map", lbox_tuple_to_map},
> - {"tuple_field_by_path", lbox_tuple_field_by_path},
> - {NULL, NULL}
> + { "__gc", lbox_tuple_gc },
> + { "tostring", lbox_tuple_to_string },
> + { "slice", lbox_tuple_slice },
> + { "transform", lbox_tuple_transform },
> + { "tuple_to_map", lbox_tuple_to_map },
> + { "tuple_field_by_path", lbox_tuple_field_by_path },
> + { NULL, NULL }
> };
>
> -static const struct luaL_Reg lbox_tuplelib[] = {
> - {"new", lbox_tuple_new},
> - {NULL, NULL}
> -};
> +static const struct luaL_Reg lbox_tuplelib[] = { { "new", lbox_tuple_new },
> + { NULL, NULL } };
91. Ugly.
>
> -static const struct luaL_Reg lbox_tuple_iterator_meta[] = {
> - {NULL, NULL}
> -};
> +static const struct luaL_Reg lbox_tuple_iterator_meta[] = { { NULL, NULL } };
>
> /* }}} */
> diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
> index 2283a471d..69294f060 100644
> --- a/src/box/memtx_bitset.c
> +++ b/src/box/memtx_bitset.c
> @@ -490,7 +494,7 @@ static const struct index_vtab memtx_bitset_index_vtab = {
> /* .update_def = */ generic_index_update_def,
> /* .depends_on_pk = */ generic_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_bitset_index_size,
> /* .bsize = */ memtx_bitset_index_bsize,
> /* .min = */ generic_index_min,
> @@ -501,7 +505,7 @@ static const struct index_vtab memtx_bitset_index_vtab = {
> /* .replace = */ memtx_bitset_index_replace,
> /* .create_iterator = */ memtx_bitset_index_create_iterator,
> /* .create_snapshot_iterator = */
> - generic_index_create_snapshot_iterator,
> + generic_index_create_snapshot_iterator,
92. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
> index 43000ba0b..485361532 100644
> --- a/src/box/memtx_engine.c
> +++ b/src/box/memtx_engine.c
> @@ -170,8 +170,8 @@ memtx_engine_recover_snapshot(struct memtx_engine *memtx,
> int rc;
> struct xrow_header row;
> uint64_t row_count = 0;
> - while ((rc = xlog_cursor_next(&cursor, &row,
> - memtx->force_recovery)) == 0) {
> + while ((rc = xlog_cursor_next(&cursor, &row, memtx->force_recovery)) ==
> + 0) {
93. Ugly.
> row.lsn = signature;
> rc = memtx_engine_recover_snapshot_row(memtx, &row);
> if (rc < 0) {
> @@ -287,8 +286,8 @@ memtx_engine_begin_initial_recovery(struct engine *engine,
> * recovery mode. Enable all keys on start, to detect and
> * discard duplicates in the snapshot.
> */
> - memtx->state = (memtx->force_recovery ?
> - MEMTX_OK : MEMTX_INITIAL_RECOVERY);
> + memtx->state = (memtx->force_recovery ? MEMTX_OK :
> + MEMTX_INITIAL_RECOVERY);
94. Ugly and random.
> return 0;
> }
>
> @@ -365,7 +364,8 @@ memtx_engine_prepare(struct engine *engine, struct txn *txn)
> {
> (void)engine;
> struct txn_stmt *stmt;
> - stailq_foreach_entry(stmt, &txn->stmts, next) {
> + stailq_foreach_entry(stmt, &txn->stmts, next)
> + {
95. This is a for loop and should be formatted like a for loop.
> if (stmt->add_story != NULL || stmt->del_story != NULL)
> memtx_tx_history_prepare_stmt(stmt);
> }
> @@ -377,7 +377,8 @@ memtx_engine_commit(struct engine *engine, struct txn *txn)
> {
> (void)engine;
> struct txn_stmt *stmt;
> - stailq_foreach_entry(stmt, &txn->stmts, next) {
> + stailq_foreach_entry(stmt, &txn->stmts, next)
> + {
96. Ditto.
> if (stmt->add_story != NULL || stmt->del_story != NULL) {
> ssize_t bsize = memtx_tx_history_commit_stmt(stmt);
> assert(stmt->space->engine == engine);
> diff --git a/src/box/memtx_engine.h b/src/box/memtx_engine.h
> index 8b380bf3c..a03305580 100644
> --- a/src/box/memtx_engine.h
> +++ b/src/box/memtx_engine.h
> @@ -256,10 +255,7 @@ memtx_tuple_delete(struct tuple_format *format, struct tuple *tuple);
> /** Tuple format vtab for memtx engine. */
> extern struct tuple_format_vtab memtx_tuple_format_vtab;
>
> -enum {
> - MEMTX_EXTENT_SIZE = 16 * 1024,
> - MEMTX_SLAB_SIZE = 4 * 1024 * 1024
> -};
> +enum { MEMTX_EXTENT_SIZE = 16 * 1024, MEMTX_SLAB_SIZE = 4 * 1024 * 1024 };
97. Ugly. Harder to read, harder to extend.
>
> /**
> * Allocate a block of size MEMTX_EXTENT_SIZE for memtx index
> diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
> index ed4dba90a..ec143ae5d 100644
> --- a/src/box/memtx_hash.c
> +++ b/src/box/memtx_hash.c
> @@ -46,16 +46,17 @@ static inline bool
> memtx_hash_equal(struct tuple *tuple_a, struct tuple *tuple_b,
> struct key_def *key_def)
> {
> - return tuple_compare(tuple_a, HINT_NONE,
> - tuple_b, HINT_NONE, key_def) == 0;
> + return tuple_compare(tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def) ==
> + 0;
98. Ugly.
> }
>
> static inline bool
> memtx_hash_equal_key(struct tuple *tuple, const char *key,
> struct key_def *key_def)
> {
> - return tuple_compare_with_key(tuple, HINT_NONE, key, key_def->part_count,
> - HINT_NONE, key_def) == 0;
> + return tuple_compare_with_key(tuple, HINT_NONE, key,
> + key_def->part_count, HINT_NONE,
> + key_def) == 0;
> }
> @@ -130,26 +131,27 @@ hash_iterator_gt_base(struct iterator *ptr, struct tuple **ret)
> return 0;
> }
>
> -#define WRAP_ITERATOR_METHOD(name) \
> -static int \
> -name(struct iterator *iterator, struct tuple **ret) \
> -{ \
> - struct txn *txn = in_txn(); \
> - struct space *space = space_by_id(iterator->space_id); \
> - bool is_rw = txn != NULL; \
> - uint32_t iid = iterator->index->def->iid; \
> - bool is_first = true; \
> - do { \
> - int rc = is_first ? name##_base(iterator, ret) \
> - : hash_iterator_ge_base(iterator, ret); \
> - if (rc != 0 || *ret == NULL) \
> - return rc; \
> - is_first = false; \
> - *ret = memtx_tx_tuple_clarify(txn, space, *ret, iid, 0, is_rw); \
> - } while (*ret == NULL); \
> - return 0; \
> -} \
> -struct forgot_to_add_semicolon
> +#define WRAP_ITERATOR_METHOD(name) \
> + static int name(struct iterator *iterator, struct tuple **ret) \
99. Return type should be on separate line. Also we don't add +1 tab inside
macros, intentionally.
> + { \
> + struct txn *txn = in_txn(); \
> + struct space *space = space_by_id(iterator->space_id); \
> + bool is_rw = txn != NULL; \
> + uint32_t iid = iterator->index->def->iid; \
> + bool is_first = true; \
> + do { \
> + int rc = is_first ? \
> + name##_base(iterator, ret) : \
> + hash_iterator_ge_base(iterator, ret); \
100. Ugly and random.
> + if (rc != 0 || *ret == NULL) \
> + return rc; \
> + is_first = false; \
> + *ret = memtx_tx_tuple_clarify(txn, space, *ret, iid, \
> + 0, is_rw); \
> + } while (*ret == NULL); \
> + return 0; \
> + } \
> + struct forgot_to_add_semicolon
>
> @@ -536,7 +542,7 @@ static const struct index_vtab memtx_hash_index_vtab = {
> /* .update_def = */ memtx_hash_index_update_def,
> /* .depends_on_pk = */ generic_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_hash_index_size,
> /* .bsize = */ memtx_hash_index_bsize,
> /* .min = */ generic_index_min,
> @@ -547,7 +553,7 @@ static const struct index_vtab memtx_hash_index_vtab = {
> /* .replace = */ memtx_hash_index_replace,
> /* .create_iterator = */ memtx_hash_index_create_iterator,
> /* .create_snapshot_iterator = */
> - memtx_hash_index_create_snapshot_iterator,
> + memtx_hash_index_create_snapshot_iterator,
101. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
> index b734daa56..80db1293f 100644
> --- a/src/box/memtx_rtree.c
> +++ b/src/box/memtx_rtree.c
> @@ -368,7 +370,7 @@ static const struct index_vtab memtx_rtree_index_vtab = {
> /* .update_def = */ generic_index_update_def,
> /* .depends_on_pk = */ generic_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_rtree_index_def_change_requires_rebuild,
> + memtx_rtree_index_def_change_requires_rebuild,
> /* .size = */ memtx_rtree_index_size,
> /* .bsize = */ memtx_rtree_index_bsize,
> /* .min = */ generic_index_min,
> @@ -379,7 +381,7 @@ static const struct index_vtab memtx_rtree_index_vtab = {
> /* .replace = */ memtx_rtree_index_replace,
> /* .create_iterator = */ memtx_rtree_index_create_iterator,
> /* .create_snapshot_iterator = */
> - generic_index_create_snapshot_iterator,
> + generic_index_create_snapshot_iterator,
102. Ditto.
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 2a43e64bf..17774f3ba 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -242,17 +242,17 @@ memtx_space_replace_primary_key(struct space *space, struct tuple *old_tuple,
> int
> memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple,
> struct tuple *new_tuple,
> - enum dup_replace_mode mode,
> - struct tuple **result)
> + enum dup_replace_mode mode, struct tuple **result)
> {
> struct memtx_engine *memtx = (struct memtx_engine *)space->engine;
> /*
> * Ensure we have enough slack memory to guarantee
> * successful statement-level rollback.
> */
> - if (memtx_index_extent_reserve(memtx, new_tuple != NULL ?
> - RESERVE_EXTENTS_BEFORE_REPLACE :
> - RESERVE_EXTENTS_BEFORE_DELETE) != 0)
> + if (memtx_index_extent_reserve(
> + memtx, new_tuple != NULL ?
> + RESERVE_EXTENTS_BEFORE_REPLACE :
> + RESERVE_EXTENTS_BEFORE_DELETE) != 0)
103. Ugly and random.
> return -1;
>
> uint32_t i = 0;
> @@ -988,7 +984,7 @@ memtx_build_on_replace(struct trigger *trigger, void *event)
> struct txn_stmt *stmt = txn_current_stmt(txn);
>
> struct tuple *cmp_tuple = stmt->new_tuple != NULL ? stmt->new_tuple :
> - stmt->old_tuple;
> + stmt->old_tuple;
104. Ugly and random. It seems to me now, that this shit tries to
align the second line by '->' on the previous line.
> /*
> diff --git a/src/box/memtx_tree.cc b/src/box/memtx_tree.cc
> index 44bdc86e6..ab233a70e 100644
> --- a/src/box/memtx_tree.cc
> +++ b/src/box/memtx_tree.cc
> @@ -52,17 +52,14 @@ struct memtx_tree_key_data_common {
> uint32_t part_count;
> };
>
> -template <bool USE_HINT>
> -struct memtx_tree_key_data;
> +template <bool USE_HINT> struct memtx_tree_key_data;
105. Template on the same line as its type looks harder to read,
and harder to extend with new arguments.
>
> -template <>
> -struct memtx_tree_key_data<false> : memtx_tree_key_data_common {
> +template <> struct memtx_tree_key_data<false>: memtx_tree_key_data_common {
> static constexpr hint_t hint = HINT_NONE;
> void set_hint(hint_t) { assert(false); }
> };
> @@ -407,41 +398,42 @@ tree_iterator_prev_equal_base(struct iterator *iterator, struct tuple **ret)
> return 0;
> }
>
> -#define WRAP_ITERATOR_METHOD(name) \
> -template <bool USE_HINT> \
> -static int \
> -name(struct iterator *iterator, struct tuple **ret) \
> -{ \
> - memtx_tree_t<USE_HINT> *tree = \
> - &((struct memtx_tree_index<USE_HINT> *)iterator->index)->tree; \
> - struct tree_iterator<USE_HINT> *it = \
> - get_tree_iterator<USE_HINT>(iterator); \
> - memtx_tree_iterator_t<USE_HINT> *ti = &it->tree_iterator; \
> - uint32_t iid = iterator->index->def->iid; \
> - bool is_multikey = iterator->index->def->key_def->is_multikey; \
> - struct txn *txn = in_txn(); \
> - struct space *space = space_by_id(iterator->space_id); \
> - bool is_rw = txn != NULL; \
> - do { \
> - int rc = name##_base<USE_HINT>(iterator, ret); \
> - if (rc != 0 || *ret == NULL) \
> - return rc; \
> - uint32_t mk_index = 0; \
> - if (is_multikey) { \
> - struct memtx_tree_data<USE_HINT> *check = \
> - memtx_tree_iterator_get_elem(tree, ti); \
> - assert(check != NULL); \
> - mk_index = (uint32_t)check->hint; \
> - } \
> - *ret = memtx_tx_tuple_clarify(txn, space, *ret, \
> - iid, mk_index, is_rw); \
> - } while (*ret == NULL); \
> - tuple_unref(it->current.tuple); \
> - it->current.tuple = *ret; \
> - tuple_ref(it->current.tuple); \
> - return 0; \
> -} \
> -struct forgot_to_add_semicolon
> +#define WRAP_ITERATOR_METHOD(name) \
> + template <bool USE_HINT> \
> + static int name(struct iterator *iterator, struct tuple **ret) \
> + { \
> + memtx_tree_t<USE_HINT> *tree = \
> + &((struct memtx_tree_index<USE_HINT> *)iterator->index) \
> + ->tree; \
> + struct tree_iterator<USE_HINT> *it = \
> + get_tree_iterator<USE_HINT>(iterator); \
> + memtx_tree_iterator_t<USE_HINT> *ti = &it->tree_iterator; \
> + uint32_t iid = iterator->index->def->iid; \
> + bool is_multikey = iterator->index->def->key_def->is_multikey; \
> + struct txn *txn = in_txn(); \
> + struct space *space = space_by_id(iterator->space_id); \
> + bool is_rw = txn != NULL; \
> + do { \
> + int rc = name##_base<USE_HINT>(iterator, ret); \
> + if (rc != 0 || *ret == NULL) \
> + return rc; \
> + uint32_t mk_index = 0; \
> + if (is_multikey) { \
> + struct memtx_tree_data<USE_HINT> *check = \
> + memtx_tree_iterator_get_elem(tree, \
> + ti); \
> + assert(check != NULL); \
> + mk_index = (uint32_t)check->hint; \
> + } \
> + *ret = memtx_tx_tuple_clarify(txn, space, *ret, iid, \
> + mk_index, is_rw); \
> + } while (*ret == NULL); \
> + tuple_unref(it->current.tuple); \
> + it->current.tuple = *ret; \
> + tuple_ref(it->current.tuple); \
> + return 0; \
> + } \
106. Return type should be one separate line. And we don't
add +1 tab inside macros.
> + struct forgot_to_add_semicolon
>
> WRAP_ITERATOR_METHOD(tree_iterator_next);
> WRAP_ITERATOR_METHOD(tree_iterator_prev);
> @@ -1529,7 +1528,7 @@ static const struct index_vtab memtx_tree_no_hint_index_vtab = {
> /* .update_def = */ memtx_tree_index_update_def<false>,
> /* .depends_on_pk = */ memtx_tree_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_tree_index_size<false>,
> /* .bsize = */ memtx_tree_index_bsize<false>,
> /* .min = */ generic_index_min,
> @@ -1540,7 +1539,7 @@ static const struct index_vtab memtx_tree_no_hint_index_vtab = {
> /* .replace = */ memtx_tree_index_replace<false>,
> /* .create_iterator = */ memtx_tree_index_create_iterator<false>,
> /* .create_snapshot_iterator = */
> - memtx_tree_index_create_snapshot_iterator<false>,
> + memtx_tree_index_create_snapshot_iterator<false>,
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> @@ -1559,7 +1558,7 @@ static const struct index_vtab memtx_tree_use_hint_index_vtab = {
> /* .update_def = */ memtx_tree_index_update_def<true>,
> /* .depends_on_pk = */ memtx_tree_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_tree_index_size<true>,
> /* .bsize = */ memtx_tree_index_bsize<true>,
> /* .min = */ generic_index_min,
> @@ -1570,7 +1569,7 @@ static const struct index_vtab memtx_tree_use_hint_index_vtab = {
> /* .replace = */ memtx_tree_index_replace<true>,
> /* .create_iterator = */ memtx_tree_index_create_iterator<true>,
> /* .create_snapshot_iterator = */
> - memtx_tree_index_create_snapshot_iterator<true>,
> + memtx_tree_index_create_snapshot_iterator<true>,
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> @@ -1589,7 +1588,7 @@ static const struct index_vtab memtx_tree_index_multikey_vtab = {
> /* .update_def = */ memtx_tree_index_update_def<true>,
> /* .depends_on_pk = */ memtx_tree_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_tree_index_size<true>,
> /* .bsize = */ memtx_tree_index_bsize<true>,
> /* .min = */ generic_index_min,
> @@ -1600,7 +1599,7 @@ static const struct index_vtab memtx_tree_index_multikey_vtab = {
> /* .replace = */ memtx_tree_index_replace_multikey,
> /* .create_iterator = */ memtx_tree_index_create_iterator<true>,
> /* .create_snapshot_iterator = */
> - memtx_tree_index_create_snapshot_iterator<true>,
> + memtx_tree_index_create_snapshot_iterator<true>,
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> @@ -1619,7 +1618,7 @@ static const struct index_vtab memtx_tree_func_index_vtab = {
> /* .update_def = */ memtx_tree_index_update_def<true>,
> /* .depends_on_pk = */ memtx_tree_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - memtx_index_def_change_requires_rebuild,
> + memtx_index_def_change_requires_rebuild,
> /* .size = */ memtx_tree_index_size<true>,
> /* .bsize = */ memtx_tree_index_bsize<true>,
> /* .min = */ generic_index_min,
> @@ -1630,7 +1629,7 @@ static const struct index_vtab memtx_tree_func_index_vtab = {
> /* .replace = */ memtx_tree_func_index_replace,
> /* .create_iterator = */ memtx_tree_index_create_iterator<true>,
> /* .create_snapshot_iterator = */
> - memtx_tree_index_create_snapshot_iterator<true>,
> + memtx_tree_index_create_snapshot_iterator<true>,
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> @@ -1655,7 +1654,7 @@ static const struct index_vtab memtx_tree_disabled_index_vtab = {
> /* .update_def = */ generic_index_update_def,
> /* .depends_on_pk = */ generic_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - generic_index_def_change_requires_rebuild,
> + generic_index_def_change_requires_rebuild,
> /* .size = */ generic_index_size,
> /* .bsize = */ generic_index_bsize,
> /* .min = */ generic_index_min,
> @@ -1666,7 +1665,7 @@ static const struct index_vtab memtx_tree_disabled_index_vtab = {
> /* .replace = */ disabled_index_replace,
> /* .create_iterator = */ generic_index_create_iterator,
> /* .create_snapshot_iterator = */
> - generic_index_create_snapshot_iterator,
> + generic_index_create_snapshot_iterator,
107. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> @@ -1682,15 +1681,14 @@ memtx_tree_index_new_tpl(struct memtx_engine *memtx, struct index_def *def,
> const struct index_vtab *vtab)
> {
> struct memtx_tree_index<USE_HINT> *index =
> - (struct memtx_tree_index<USE_HINT> *)
> - calloc(1, sizeof(*index));
> + (struct memtx_tree_index<USE_HINT> *)calloc(1, sizeof(*index));
> if (index == NULL) {
> - diag_set(OutOfMemory, sizeof(*index),
> - "malloc", "struct memtx_tree_index");
> + diag_set(OutOfMemory, sizeof(*index), "malloc",
> + "struct memtx_tree_index");
> return NULL;
> }
> - if (index_create(&index->base, (struct engine *)memtx,
> - vtab, def) != 0) {
> + if (index_create(&index->base, (struct engine *)memtx, vtab, def) !=
> + 0) {
108. Ugly.
> free(index);
> return NULL;
> diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c
> index bcd4a377c..2eae5ad01 100644
> --- a/src/box/memtx_tx.c
> +++ b/src/box/memtx_tx.c
> @@ -309,8 +306,8 @@ memtx_tx_story_get(struct tuple *tuple)
> static struct tuple *
> memtx_tx_story_older_tuple(struct memtx_story_link *link)
> {
> - return link->older.is_story ? link->older.story->tuple
> - : link->older.tuple;
> + return link->older.is_story ? link->older.story->tuple :
> + link->older.tuple;
109. Ugly and random.
> }
> merger_new(struct key_def *key_def, struct merge_source **sources,
> uint32_t source_count, bool reverse)
> diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc
> index 36fbcefc5..dd4189cde 100644
> --- a/src/box/mp_error.cc
> +++ b/src/box/mp_error.cc
> @@ -253,7 +246,7 @@ error_build_xc(struct mp_error *mp_error)
> struct error *err = NULL;
> if (mp_error->type == NULL || mp_error->message == NULL ||
> mp_error->file == NULL) {
> -missing_fields:
> + missing_fields:
110. Labels are visible in the whole function, and should not have
indentation.
> diag_set(ClientError, ER_INVALID_MSGPACK,
> "Missing mandatory error fields");
> @@ -347,7 +340,8 @@ mp_decode_and_copy_str(const char **data, struct region *region)
> }
> uint32_t str_len;
> const char *str = mp_decode_str(data, &str_len);
> - return region_strdup(region, str, str_len);;
> + return region_strdup(region, str, str_len);
> + ;
111. Another proof, that you didn't even open your patch, and decided
that better a reviewer will waste time on this.
> }
>
> static inline bool
> @@ -579,7 +573,7 @@ error_unpack_unsafe(const char **data)
> #define MP_ERROR_PRINT_DEFINITION
> #define MP_PRINT_FUNC snprintf
> #define MP_PRINT_SUFFIX snprint
> -#define MP_PRINT_2(total, func, ...) \
> +#define MP_PRINT_2(total, func, ...) \
> SNPRINT(total, func, buf, size, __VA_ARGS__)
> #define MP_PRINT_ARGS_DECL char *buf, int size
> #include __FILE__
> @@ -587,12 +581,13 @@ error_unpack_unsafe(const char **data)
> #define MP_ERROR_PRINT_DEFINITION
> #define MP_PRINT_FUNC fprintf
> #define MP_PRINT_SUFFIX fprint
> -#define MP_PRINT_2(total, func, ...) do { \
> - int bytes = func(file, __VA_ARGS__); \
> - if (bytes < 0) \
> - return -1; \
> - total += bytes; \
> -} while (0)
> +#define MP_PRINT_2(total, func, ...) \
> + do { \
> + int bytes = func(file, __VA_ARGS__); \
> + if (bytes < 0) \
> + return -1; \
> + total += bytes; \
> + } while (0)
112. The \ signs were aligned with advance, so as we could add more lines,
which can be longer, and it won't make us change the other lines by
moving the other \ symbols. Now it is broken.
> #define MP_PRINT_ARGS_DECL FILE *file
> #include __FILE__
> diff --git a/src/box/opt_def.h b/src/box/opt_def.h
> index 21544412c..6640678f8 100644
> --- a/src/box/opt_def.h
> +++ b/src/box/opt_def.h
> @@ -40,15 +40,15 @@ extern "C" {
> #endif /* defined(__cplusplus) */
>
> enum opt_type {
> - OPT_BOOL, /* bool */
> - OPT_UINT32, /* uint32_t */
> - OPT_INT64, /* int64_t */
> - OPT_FLOAT, /* double */
> - OPT_STR, /* char[] */
> - OPT_STRPTR, /* char* */
> - OPT_ENUM, /* enum */
> - OPT_ARRAY, /* array */
> - OPT_LEGACY, /* any type, skipped */
> + OPT_BOOL, /* bool */
> + OPT_UINT32, /* uint32_t */
> + OPT_INT64, /* int64_t */
> + OPT_FLOAT, /* double */
> + OPT_STR, /* char[] */
> + OPT_STRPTR, /* char* */
> + OPT_ENUM, /* enum */
> + OPT_ARRAY, /* array */
> + OPT_LEGACY, /* any type, skipped */
> opt_type_MAX,
113. Ditto.
> };
>
> static_assert(sizeof(struct port_vdbemem) <= sizeof(struct port),
> diff --git a/src/box/raft.c b/src/box/raft.c
> index 4a8e54cac..2d32a8def 100644
> --- a/src/box/raft.c
> +++ b/src/box/raft.c
> @@ -581,7 +585,7 @@ raft_worker_handle_io(void)
> struct raft_request req;
>
> if (raft_is_fully_on_disk()) {
> -end_dump:
> + end_dump:
114. Labels are visible everywhere inside a function, and should not
have indentation.
> raft.is_write_in_progress = false;
> /*
> * The state machine is stable. Can see now, to what state to
> diff --git a/src/box/recovery.cc b/src/box/recovery.cc
> index cd33e7635..35693b9e2 100644
> --- a/src/box/recovery.cc
> +++ b/src/box/recovery.cc
> @@ -345,7 +339,7 @@ recover_remaining_wals(struct recovery *r, struct xstream *stream,
>
> say_info("recover from `%s'", r->cursor.name);
>
> -recover_current_wal:
> + recover_current_wal:
115. Ditto.
> recover_xlog(r, stream, stop_vclock);
> }
> @@ -491,7 +480,8 @@ hot_standby_f(va_list ap)
> } while (end > start && !xlog_cursor_is_open(&r->cursor));
>
> subscription.set_log_path(xlog_cursor_is_open(&r->cursor) ?
> - r->cursor.name : NULL);
> + r->cursor.name :
> + NULL);
116. Ugly and random.
>
> bool timed_out = false;
> if (subscription.events == 0) {
> diff --git a/src/box/relay.cc b/src/box/relay.cc
> index b68b45e00..38eb39bfd 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -144,8 +144,8 @@ struct relay {
> struct {
> /* Align to prevent false-sharing with tx thread */
> alignas(CACHELINE_SIZE)
> - /** Known relay vclock. */
> - struct vclock vclock;
> + /** Known relay vclock. */
> + struct vclock vclock;
117. Wtf?
> /**
> * True if the relay needs Raft updates. It can live fine
> * without sending Raft updates, if it is a relay to an
> @@ -256,8 +256,9 @@ static void
> relay_stop(struct relay *relay)
> {
> struct relay_gc_msg *gc_msg, *next_gc_msg;
> - stailq_foreach_entry_safe(gc_msg, next_gc_msg,
> - &relay->pending_gc, in_pending) {
> + stailq_foreach_entry_safe(gc_msg, next_gc_msg, &relay->pending_gc,
> + in_pending)
> + {
118. This is a for loop and is supposed to be formatted like a
for loop.
> free(gc_msg);
> }
> @@ -423,9 +423,8 @@ tx_status_update(struct cmsg *msg)
> txn_limbo_ack(&txn_limbo, status->relay->replica->id,
> vclock_get(&status->vclock, instance_id));
> }
> - static const struct cmsg_hop route[] = {
> - {relay_status_update, NULL}
> - };
> + static const struct cmsg_hop route[] = { { relay_status_update,
> + NULL } };
119. Ugly.
> cmsg_init(msg, route);
> cpipe_push(&status->relay->relay_pipe, msg);
> }
> @@ -477,7 +474,8 @@ static inline void
> relay_schedule_pending_gc(struct relay *relay, const struct vclock *vclock)
> {
> struct relay_gc_msg *curr, *next, *gc_msg = NULL;
> - stailq_foreach_entry_safe(curr, next, &relay->pending_gc, in_pending) {
> + stailq_foreach_entry_safe(curr, next, &relay->pending_gc, in_pending)
> + {
120. This is a for loop and is supposed to be formatted like a
for loop.
> /*
> * We may delete a WAL file only if its vclock is
> * less than or equal to the vclock acknowledged by
> @@ -741,9 +740,8 @@ relay_subscribe_f(va_list ap)
> if (vclock_sum(&relay->status_msg.vclock) ==
> vclock_sum(send_vclock))
> continue;
> - static const struct cmsg_hop route[] = {
> - {tx_status_update, NULL}
> - };
> + static const struct cmsg_hop route[] = { { tx_status_update,
> + NULL } };
121. Ugly.
> cmsg_init(&relay->status_msg.msg, route);
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index c19f8c693..0715bfece 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -508,8 +505,9 @@ replicaset_update(struct applier **appliers, int count)
> struct replica *replica, *next;
> struct applier *applier;
>
> - auto uniq_guard = make_scoped_guard([&]{
> - replica_hash_foreach_safe(&uniq, replica, next) {
> + auto uniq_guard = make_scoped_guard([&] {
> + replica_hash_foreach_safe(&uniq, replica, next)
> + {
122. This is a for loop and is supposed to be formatted like a
for loop.
> replica_hash_remove(&uniq, replica);
> replica_clear_applier(replica);
> replica_delete(replica);
> @@ -564,7 +562,8 @@ replicaset_update(struct applier **appliers, int count)
> applier_stop(applier);
> applier_delete(applier);
> }
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
123. Ditto.
> if (replica->applier == NULL)
> continue;
> applier = replica->applier;
> @@ -580,7 +579,8 @@ replicaset_update(struct applier **appliers, int count)
> replicaset.applier.loading = 0;
> replicaset.applier.synced = 0;
>
> - replica_hash_foreach_safe(&uniq, replica, next) {
> + replica_hash_foreach_safe(&uniq, replica, next)
> + {
124. Ditto.
> replica_hash_remove(&uniq, replica);
>
> struct replica *orig = replica_hash_search(&replicaset.hash,
> @@ -603,7 +603,8 @@ replicaset_update(struct applier **appliers, int count)
> rlist_swap(&replicaset.anon, &anon_replicas);
>
> assert(replica_hash_first(&uniq) == NULL);
> - replica_hash_foreach_safe(&replicaset.hash, replica, next) {
> + replica_hash_foreach_safe(&replicaset.hash, replica, next)
> + {
125. Ditto.
> if (replica_is_orphan(replica)) {
> replica_hash_remove(&replicaset.hash, replica);
> replicaset.anon_count -= replica->anon;
> @@ -782,19 +783,20 @@ bool
> replicaset_needs_rejoin(struct replica **master)
> {
> struct replica *leader = NULL;
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
126. Ditto.
> struct applier *applier = replica->applier;
> /*
> * Skip the local instance, we shouldn't perform a
> * check against our own gc vclock.
> */
> - if (applier == NULL || tt_uuid_is_equal(&replica->uuid,
> - &INSTANCE_UUID))
> + if (applier == NULL ||
> + tt_uuid_is_equal(&replica->uuid, &INSTANCE_UUID))
> continue;
>
> const struct ballot *ballot = &applier->ballot;
> - if (vclock_compare(&ballot->gc_vclock,
> - &replicaset.vclock) <= 0) {
> + if (vclock_compare(&ballot->gc_vclock, &replicaset.vclock) <=
> + 0) {
127. Ugly.
> /*
> * There's at least one master that still stores
> * WALs needed by this instance. Proceed to local
> @@ -843,7 +848,8 @@ void
> replicaset_follow(void)
> {
> struct replica *replica;
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
128. This is a for loop and is supposed to be formatted like a
for loop.
> /* Resume connected appliers. */
> if (replica->applier != NULL)
> applier_resume(replica->applier);
> @@ -957,7 +963,8 @@ static struct replica *
> replicaset_round(bool skip_ro)
> {
> struct replica *leader = NULL;
> - replicaset_foreach(replica) {
> + replicaset_foreach(replica)
> + {
129. Ditto.
> struct applier *applier = replica->applier;
> if (applier == NULL)
> continue;
> @@ -986,12 +994,13 @@ replicaset_round(bool skip_ro)
> * with the same vclock, prefer the one with
> * the lowest uuid.
> */
> - int cmp = vclock_compare_ignore0(&applier->ballot.vclock,
> - &leader->applier->ballot.vclock);
> + int cmp =
> + vclock_compare_ignore0(&applier->ballot.vclock,
> + &leader->applier->ballot.vclock);
130. Ugly.
> if (cmp < 0)
> continue;
> - if (cmp == 0 && tt_uuid_compare(&replica->uuid,
> - &leader->uuid) > 0)
> + if (cmp == 0 &&
> + tt_uuid_compare(&replica->uuid, &leader->uuid) > 0)
> continue;
> leader = replica;
> diff --git a/src/box/schema.cc b/src/box/schema.cc
> index 60e4a7f9c..4fede17e4 100644
> --- a/src/box/schema.cc
> +++ b/src/box/schema.cc
> @@ -133,20 +133,21 @@ space_foreach(int (*func)(struct space *sp, void *udata), void *udata)
> space = space_by_id(BOX_SPACE_ID);
> struct index *pk = space ? space_index(space, 0) : NULL;
> if (pk) {
> - struct iterator *it = index_create_iterator(pk, ITER_GE,
> - key, 1);
> + struct iterator *it = index_create_iterator(pk, ITER_GE, key,
> + 1);
> if (it == NULL)
> return -1;
> int rc;
> struct tuple *tuple;
> while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
> uint32_t id;
> - if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &id) != 0)
> + if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &id) !=
> + 0)
131. Ugly.
> continue;
> space = space_cache_find(id);
> if (space == NULL)
> continue;
> - if (! space_is_system(space))
> + if (!space_is_system(space))
> break;
> rc = func(space, udata);
> if (rc != 0)
> @@ -157,8 +158,9 @@ space_foreach(int (*func)(struct space *sp, void *udata), void *udata)
> return -1;
> }
>
> - mh_foreach(spaces, i) {
> - space = (struct space *) mh_i32ptr_node(spaces, i)->val;
> + mh_foreach(spaces, i)
> + {
132. This is a for loop and is supposed to be formatted like a
for loop.
> @@ -261,27 +265,23 @@ space_cache_replace(struct space *old_space, struct space *new_space)
>
> /** A wrapper around space_new() for data dictionary spaces. */
> static void
> -sc_space_new(uint32_t id, const char *name,
> - struct key_part_def *key_parts,
> - uint32_t key_part_count,
> - struct trigger *replace_trigger)
> +sc_space_new(uint32_t id, const char *name, struct key_part_def *key_parts,
> + uint32_t key_part_count, struct trigger *replace_trigger)
> {
> struct key_def *key_def = key_def_new(key_parts, key_part_count, false);
> if (key_def == NULL)
> diag_raise();
> - auto key_def_guard =
> - make_scoped_guard([=] { key_def_delete(key_def); });
> - struct index_def *index_def = index_def_new(id, /* space id */
> - 0 /* index id */,
> - "primary", /* name */
> - strlen("primary"),
> - TREE /* index type */,
> - &index_opts_default,
> - key_def, NULL);
> + auto key_def_guard = make_scoped_guard(
> + [=] { key_def_delete(key_def); });
> + struct index_def *index_def =
> + index_def_new(id, /* space id */
> + 0 /* index id */, "primary", /* name */
> + strlen("primary"), TREE /* index type */,
> + &index_opts_default, key_def, NULL);
133. The previous code wasn't in our code style, but it was readable.
One line for one comment and one argument. Now it is a mess.
> if (index_def == NULL)
> diag_raise();
> - auto index_def_guard =
> - make_scoped_guard([=] { index_def_delete(index_def); });
> + auto index_def_guard = make_scoped_guard(
> + [=] { index_def_delete(index_def); });
> struct space_def *def =
> @@ -550,14 +549,15 @@ func_cache_insert(struct func *func)
> const struct mh_i32ptr_node_t node = { func->def->fid, func };
> mh_int_t k1 = mh_i32ptr_put(funcs, &node, NULL, NULL);
> if (k1 == mh_end(funcs)) {
> -error:
> + error:
134. Labels are visible everywhere inside a function, and should not
have indentation.
> panic_syserror("Out of memory for the data "
> "dictionary cache (stored function).");
> }
> diff --git a/src/box/session.cc b/src/box/session.cc
> index 7ba7235fe..d5bb1eeba 100644
> --- a/src/box/session.cc
> +++ b/src/box/session.cc
> @@ -39,12 +39,7 @@
> #include "sql_stmt_cache.h"
>
> const char *session_type_strs[] = {
> - "background",
> - "binary",
> - "console",
> - "repl",
> - "applier",
> - "unknown",
> + "background", "binary", "console", "repl", "applier", "unknown",
125. Such changes just destroy the history, and will continue destroy
it further, because any new value added will change history of all the
other values.
> diff --git a/src/box/session_settings.c b/src/box/session_settings.c
> index dbbbf2461..a2a9fcee4 100644
> --- a/src/box/session_settings.c
> +++ b/src/box/session_settings.c
> @@ -265,7 +258,7 @@ static const struct index_vtab session_settings_index_vtab = {
> /* .update_def = */ generic_index_update_def,
> /* .depends_on_pk = */ generic_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - generic_index_def_change_requires_rebuild,
> + generic_index_def_change_requires_rebuild,
> /* .size = */ generic_index_size,
> /* .bsize = */ generic_index_bsize,
> /* .min = */ generic_index_min,
> @@ -276,7 +269,7 @@ static const struct index_vtab session_settings_index_vtab = {
> /* .replace = */ generic_index_replace,
> /* .create_iterator = */ session_settings_index_create_iterator,
> /* .create_snapshot_iterator = */
> - generic_index_create_snapshot_iterator,
> + generic_index_create_snapshot_iterator,
126. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .stat = */ generic_index_stat,
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> diff --git a/src/box/space.c b/src/box/space.c
> index 6d1d77117..eaf7f9d1a 100644
> --- a/src/box/space.c
> +++ b/src/box/space.c
> @@ -565,13 +566,13 @@ space_execute_dml(struct space *space, struct txn *txn,
> switch (request->type) {
> case IPROTO_INSERT:
> case IPROTO_REPLACE:
> - if (space->vtab->execute_replace(space, txn,
> - request, result) != 0)
> + if (space->vtab->execute_replace(space, txn, request, result) !=
> + 0)
127. Ugly.
> return -1;
> break;
> case IPROTO_UPDATE:
> - if (space->vtab->execute_update(space, txn,
> - request, result) != 0)
> + if (space->vtab->execute_update(space, txn, request, result) !=
> + 0)
128. Ugly.
> return -1;
> if (*result != NULL && request->index_id != 0) {
> /*
> @@ -583,8 +584,8 @@ space_execute_dml(struct space *space, struct txn *txn,
> }
> break;
> case IPROTO_DELETE:
> - if (space->vtab->execute_delete(space, txn,
> - request, result) != 0)
> + if (space->vtab->execute_delete(space, txn, request, result) !=
> + 0)
129. Ugly.
> diff --git a/src/box/sysview.c b/src/box/sysview.c
> index 00c320b6f..2051c464a 100644
> --- a/src/box/sysview.c
> +++ b/src/box/sysview.c
> @@ -196,7 +196,7 @@ static const struct index_vtab sysview_index_vtab = {
> /* .replace = */ generic_index_replace,
> /* .create_iterator = */ sysview_index_create_iterator,
> /* .create_snapshot_iterator = */
> - generic_index_create_snapshot_iterator,
> + generic_index_create_snapshot_iterator,
> /* .stat = */ generic_index_stat,
130. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .compact = */ generic_index_compact,
> /* .reset_stat = */ generic_index_reset_stat,
> * Return the size of a tuple bloom filter when encoded.
> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index 0f3f18bdf..b0a8071d2 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -447,16 +437,16 @@ mp_compare_scalar_coll(const char *field_a, const char *field_b,
> * @retval >0 if field_a > field_b
> */
> static int
> -tuple_compare_field(const char *field_a, const char *field_b,
> - int8_t type, struct coll *coll)
> +tuple_compare_field(const char *field_a, const char *field_b, int8_t type,
> + struct coll *coll)
> {
> switch (type) {
> case FIELD_TYPE_UNSIGNED:
> return mp_compare_uint(field_a, field_b);
> case FIELD_TYPE_STRING:
> return coll != NULL ?
> - mp_compare_str_coll(field_a, field_b, coll) :
> - mp_compare_str(field_a, field_b);
> + mp_compare_str_coll(field_a, field_b, coll) :
> + mp_compare_str(field_a, field_b);
131. Ugly and random.
> case FIELD_TYPE_INTEGER:
> return mp_compare_integer_with_type(field_a,
> mp_typeof(*field_a),
> @@ -472,8 +462,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
> return mp_compare_bin(field_a, field_b);
> case FIELD_TYPE_SCALAR:
> return coll != NULL ?
> - mp_compare_scalar_coll(field_a, field_b, coll) :
> - mp_compare_scalar(field_a, field_b);
> + mp_compare_scalar_coll(field_a, field_b, coll) :
> + mp_compare_scalar(field_a, field_b);
132. Ugly and random.
> case FIELD_TYPE_DECIMAL:
> return mp_compare_decimal(field_a, field_b);
> case FIELD_TYPE_UUID:
> @@ -494,14 +484,14 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
> return mp_compare_uint(field_a, field_b);
> case FIELD_TYPE_STRING:
> return coll != NULL ?
> - mp_compare_str_coll(field_a, field_b, coll) :
> - mp_compare_str(field_a, field_b);
> + mp_compare_str_coll(field_a, field_b, coll) :
> + mp_compare_str(field_a, field_b);
133. Ugly and random.
> case FIELD_TYPE_INTEGER:
> - return mp_compare_integer_with_type(field_a, a_type,
> - field_b, b_type);
> + return mp_compare_integer_with_type(field_a, a_type, field_b,
> + b_type);
> case FIELD_TYPE_NUMBER:
> - return mp_compare_number_with_type(field_a, a_type,
> - field_b, b_type);
> + return mp_compare_number_with_type(field_a, a_type, field_b,
> + b_type);
> case FIELD_TYPE_DOUBLE:
> return mp_compare_double(field_a, field_b);
> case FIELD_TYPE_BOOLEAN:
> @@ -510,12 +500,12 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
> return mp_compare_bin(field_a, field_b);
> case FIELD_TYPE_SCALAR:
> return coll != NULL ?
> - mp_compare_scalar_coll(field_a, field_b, coll) :
> - mp_compare_scalar_with_type(field_a, a_type,
> - field_b, b_type);
> + mp_compare_scalar_coll(field_a, field_b, coll) :
> + mp_compare_scalar_with_type(field_a, a_type,
> + field_b, b_type);
134. Ugly and random.
> case FIELD_TYPE_DECIMAL:
> - return mp_compare_number_with_type(field_a, a_type,
> - field_b, b_type);
> + return mp_compare_number_with_type(field_a, a_type, field_b,
> + b_type);
> case FIELD_TYPE_UUID:
> return mp_compare_uuid(field_a, field_b);
> default:
> @@ -1036,20 +1025,17 @@ field_compare_and_next<FIELD_TYPE_STRING>(const char **field_a,
> /* Tuple comparator */
> namespace /* local symbols */ {
>
> -template <int IDX, int TYPE, int ...MORE_TYPES> struct FieldCompare { };
> +template <int IDX, int TYPE, int... MORE_TYPES> struct FieldCompare {};
>
> /**
> * Common case.
> */
> -template <int IDX, int TYPE, int IDX2, int TYPE2, int ...MORE_TYPES>
> -struct FieldCompare<IDX, TYPE, IDX2, TYPE2, MORE_TYPES...>
> -{
> - inline static int compare(struct tuple *tuple_a,
> - struct tuple *tuple_b,
> +template <int IDX, int TYPE, int IDX2, int TYPE2, int... MORE_TYPES>
> +struct FieldCompare<IDX, TYPE, IDX2, TYPE2, MORE_TYPES...> {
> + inline static int compare(struct tuple *tuple_a, struct tuple *tuple_b,
135. In the new code we put return type on separate line.
> struct tuple_format *format_a,
> struct tuple_format *format_b,
> - const char *field_a,
> - const char *field_b)
> + const char *field_a, const char *field_b)
> {
> int r;
> /* static if */
> @@ -1067,21 +1053,15 @@ struct FieldCompare<IDX, TYPE, IDX2, TYPE2, MORE_TYPES...>
> tuple_field_map(tuple_b),
> IDX2);
> }
> - return FieldCompare<IDX2, TYPE2, MORE_TYPES...>::
> - compare(tuple_a, tuple_b, format_a,
> - format_b, field_a, field_b);
> + return FieldCompare<IDX2, TYPE2, MORE_TYPES...>::compare(
> + tuple_a, tuple_b, format_a, format_b, field_a, field_b);
> }
> };
>
> -template <int IDX, int TYPE>
> -struct FieldCompare<IDX, TYPE>
> -{
> - inline static int compare(struct tuple *,
> - struct tuple *,
> - struct tuple_format *,
> - struct tuple_format *,
> - const char *field_a,
> - const char *field_b)
> +template <int IDX, int TYPE> struct FieldCompare<IDX, TYPE> {
> + inline static int compare(struct tuple *, struct tuple *,
> + struct tuple_format *, struct tuple_format *,
> + const char *field_a, const char *field_b)
136. Ditto.
> {
> return field_compare<TYPE>(&field_a, &field_b);
> }
> @@ -1226,18 +1205,17 @@ field_compare_with_key_and_next<FIELD_TYPE_STRING>(const char **field_a,
> /* Tuple with key comparator */
> namespace /* local symbols */ {
>
> -template <int FLD_ID, int IDX, int TYPE, int ...MORE_TYPES>
> +template <int FLD_ID, int IDX, int TYPE, int... MORE_TYPES>
> struct FieldCompareWithKey {};
> /**
> * common
> */
> -template <int FLD_ID, int IDX, int TYPE, int IDX2, int TYPE2, int ...MORE_TYPES>
> -struct FieldCompareWithKey<FLD_ID, IDX, TYPE, IDX2, TYPE2, MORE_TYPES...>
> -{
> - inline static int
> - compare(struct tuple *tuple, const char *key, uint32_t part_count,
> - struct key_def *key_def, struct tuple_format *format,
> - const char *field)
> +template <int FLD_ID, int IDX, int TYPE, int IDX2, int TYPE2, int... MORE_TYPES>
> +struct FieldCompareWithKey<FLD_ID, IDX, TYPE, IDX2, TYPE2, MORE_TYPES...> {
> + inline static int compare(struct tuple *tuple, const char *key,
> + uint32_t part_count, struct key_def *key_def,
> + struct tuple_format *format,
> + const char *field)
137. Ditto.
> {
> int r;
> /* static if */
> @@ -1275,13 +1253,11 @@ struct FieldCompareWithKey<FLD_ID, IDX, TYPE> {
> /**
> * header
> */
> -template <int FLD_ID, int IDX, int TYPE, int ...MORE_TYPES>
> -struct TupleCompareWithKey
> -{
> - static int
> - compare(struct tuple *tuple, hint_t tuple_hint,
> - const char *key, uint32_t part_count,
> - hint_t key_hint, struct key_def *key_def)
> +template <int FLD_ID, int IDX, int TYPE, int... MORE_TYPES>
> +struct TupleCompareWithKey {
> + static int compare(struct tuple *tuple, hint_t tuple_hint,
> + const char *key, uint32_t part_count,
> + hint_t key_hint, struct key_def *key_def)
138. Ditto.
> {
> @@ -1751,7 +1726,7 @@ field_hint_string(const char *field, struct coll *coll)
> assert(mp_typeof(*field) == MP_STR);
> uint32_t len = mp_decode_strl(&field);
> return coll == NULL ? hint_str(field, len) :
> - hint_str_coll(field, len, coll);
> + hint_str_coll(field, len, coll);
139. Ugly and random.
> }
>
> static inline hint_t
> @@ -1780,17 +1755,15 @@ field_hint_scalar(const char *field, struct coll *coll)
> case MP_STR:
> len = mp_decode_strl(&field);
> return coll == NULL ? hint_str(field, len) :
> - hint_str_coll(field, len, coll);
> + hint_str_coll(field, len, coll);
140. Ugly and random.
> case MP_BIN:
> len = mp_decode_binl(&field);
> return hint_bin(field, len);
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 9b817d3cf..7d7e4e16c 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -84,7 +83,8 @@ tuple_format_cmp(const struct tuple_format *format1,
>
> struct tuple_field *field_a;
> json_tree_foreach_entry_preorder(field_a, &a->fields.root,
> - struct tuple_field, token) {
> + struct tuple_field, token)
141. This is a for loop and is supposed to be formatted like a
for loop.
> + {
> struct tuple_field *field_b =
> tuple_format1_field_by_format2_field(b, field_a);
> if (field_a->type != field_b->type)
> @@ -115,7 +114,8 @@ tuple_format_hash(struct tuple_format *format)
> uint32_t size = 0;
> struct tuple_field *f;
> json_tree_foreach_entry_preorder(f, &format->fields.root,
> - struct tuple_field, token) {
> + struct tuple_field, token)
142. Ditto.
> + {
> TUPLE_FIELD_MEMBER_HASH(f, type, h, carry, size)
> TUPLE_FIELD_MEMBER_HASH(f, coll_id, h, carry, size)
> TUPLE_FIELD_MEMBER_HASH(f, nullable_action, h, carry, size)
> @@ -190,7 +190,8 @@ tuple_format_field_by_id(struct tuple_format *format, uint32_t id)
> {
> struct tuple_field *field;
> json_tree_foreach_entry_preorder(field, &format->fields.root,
> - struct tuple_field, token) {
> + struct tuple_field, token)
143. Ditto.
> + {
> if (field->id == id)
> return field;
> }
> @@ -492,14 +491,15 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
> size_t required_fields_sz = bitmap_size(format->total_field_count);
> format->required_fields = calloc(1, required_fields_sz);
> if (format->required_fields == NULL) {
> - diag_set(OutOfMemory, required_fields_sz,
> - "malloc", "required field bitmap");
> + diag_set(OutOfMemory, required_fields_sz, "malloc",
> + "required field bitmap");
> return -1;
> }
> struct tuple_field *field;
> uint32_t *required_fields = format->required_fields;
> json_tree_foreach_entry_preorder(field, &format->fields.root,
> - struct tuple_field, token) {
> + struct tuple_field, token)
> + {
144. Ditto.
> /*
> * In the case of the multikey index,
> * required_fields is overridden with local for
> @@ -601,7 +602,8 @@ tuple_format_destroy_fields(struct tuple_format *format)
> {
> struct tuple_field *field, *tmp;
> json_tree_foreach_entry_safe(field, &format->fields.root,
> - struct tuple_field, token, tmp) {
> + struct tuple_field, token, tmp)
> + {
145. Ditto.
> json_tree_del(&format->fields, &field->token);
> tuple_field_delete(field);
> }
> @@ -816,7 +817,8 @@ tuple_format1_can_store_format2_tuples(struct tuple_format *format1,
> return false;
> struct tuple_field *field1;
> json_tree_foreach_entry_preorder(field1, &format1->fields.root,
> - struct tuple_field, token) {
> + struct tuple_field, token)
> + {
146. Ditto.
> struct tuple_field *field2 =
> tuple_format1_field_by_format2_field(format2, field1);
> /*
> @@ -858,8 +860,8 @@ tuple_field_map_create(struct tuple_format *format, const char *tuple,
> bool validate, struct field_map_builder *builder)
> {
> struct region *region = &fiber()->gc;
> - if (field_map_builder_create(builder, format->field_map_size,
> - region) != 0)
> + if (field_map_builder_create(builder, format->field_map_size, region) !=
> + 0)
147. Ugly.
> return -1;
> diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
> index 39f89a659..3d26dd078 100644
> --- a/src/box/tuple_hash.cc
> +++ b/src/box/tuple_hash.cc
> @@ -81,30 +79,28 @@ field_hash<FIELD_TYPE_STRING>(uint32_t *ph, uint32_t *pcarry,
> return size;
> }
>
> -template <int TYPE, int ...MORE_TYPES> struct KeyFieldHash {};
> +template <int TYPE, int... MORE_TYPES> struct KeyFieldHash {};
>
> -template <int TYPE, int TYPE2, int ...MORE_TYPES>
> +template <int TYPE, int TYPE2, int... MORE_TYPES>
> struct KeyFieldHash<TYPE, TYPE2, MORE_TYPES...> {
> - static void hash(uint32_t *ph, uint32_t *pcarry,
> - const char **pfield, uint32_t *ptotal_size)
> + static void hash(uint32_t *ph, uint32_t *pcarry, const char **pfield,
> + uint32_t *ptotal_size)
148. Return type should be on separate line.
> {
> *ptotal_size += field_hash<TYPE>(ph, pcarry, pfield);
> - KeyFieldHash<TYPE2, MORE_TYPES...>::
> - hash(ph, pcarry, pfield, ptotal_size);
> + KeyFieldHash<TYPE2, MORE_TYPES...>::hash(ph, pcarry, pfield,
> + ptotal_size);
> }
> };
>
> -template <int TYPE>
> -struct KeyFieldHash<TYPE> {
> - static void hash(uint32_t *ph, uint32_t *pcarry,
> - const char **pfield, uint32_t *ptotal_size)
> +template <int TYPE> struct KeyFieldHash<TYPE> {
> + static void hash(uint32_t *ph, uint32_t *pcarry, const char **pfield,
> + uint32_t *ptotal_size)
149. Return type should be on separate line. Template too.
> {
> *ptotal_size += field_hash<TYPE>(ph, pcarry, pfield);
> }
> };
>
> -template <int TYPE, int ...MORE_TYPES>
> -struct KeyHash {
> +template <int TYPE, int... MORE_TYPES> struct KeyHash {
> static uint32_t hash(const char *key, struct key_def *)
> {
> uint32_t h = HASH_SEED;
> @@ -116,33 +112,31 @@ struct KeyHash {
> }
> };
>
> -template <>
> -struct KeyHash<FIELD_TYPE_UNSIGNED> {
> +template <> struct KeyHash<FIELD_TYPE_UNSIGNED> {
> static uint32_t hash(const char *key, struct key_def *key_def)
150. Ditto.
> {
> uint64_t val = mp_decode_uint(&key);
> - (void) key_def;
> + (void)key_def;
> if (likely(val <= UINT32_MAX))
> return val;
> - return ((uint32_t)((val)>>33^(val)^(val)<<11));
> + return ((uint32_t)((val) >> 33 ^ (val) ^ (val) << 11));
> }
> };
>
> -template <int TYPE, int ...MORE_TYPES> struct TupleFieldHash { };
> +template <int TYPE, int... MORE_TYPES> struct TupleFieldHash {};
>
> -template <int TYPE, int TYPE2, int ...MORE_TYPES>
> +template <int TYPE, int TYPE2, int... MORE_TYPES>
> struct TupleFieldHash<TYPE, TYPE2, MORE_TYPES...> {
> static void hash(const char **pfield, uint32_t *ph, uint32_t *pcarry,
> uint32_t *ptotal_size)
151. Ditto.
> {
> *ptotal_size += field_hash<TYPE>(ph, pcarry, pfield);
> - TupleFieldHash<TYPE2, MORE_TYPES...>::
> - hash(pfield, ph, pcarry, ptotal_size);
> + TupleFieldHash<TYPE2, MORE_TYPES...>::hash(pfield, ph, pcarry,
> + ptotal_size);
> }
> };
>
> -template <int TYPE>
> -struct TupleFieldHash<TYPE> {
> +template <int TYPE> struct TupleFieldHash<TYPE> {
> static void hash(const char **pfield, uint32_t *ph, uint32_t *pcarry,
> uint32_t *ptotal_size)
> {
> @@ -150,44 +144,40 @@ struct TupleFieldHash<TYPE> {
> }
> };
>
> -template <int TYPE, int ...MORE_TYPES>
> -struct TupleHash
> -{
> +template <int TYPE, int... MORE_TYPES> struct TupleHash {
> static uint32_t hash(struct tuple *tuple, struct key_def *key_def)
> {
> assert(!key_def->is_multikey);
> uint32_t h = HASH_SEED;
> uint32_t carry = 0;
> uint32_t total_size = 0;
> - const char *field = tuple_field_by_part(tuple,
> - key_def->parts,
> - MULTIKEY_NONE);
> - TupleFieldHash<TYPE, MORE_TYPES...>::
> - hash(&field, &h, &carry, &total_size);
> + const char *field = tuple_field_by_part(tuple, key_def->parts,
> + MULTIKEY_NONE);
> + TupleFieldHash<TYPE, MORE_TYPES...>::hash(&field, &h, &carry,
> + &total_size);
> return PMurHash32_Result(h, carry, total_size);
> }
> };
>
> -template <>
> -struct TupleHash<FIELD_TYPE_UNSIGNED> {
> - static uint32_t hash(struct tuple *tuple, struct key_def *key_def)
> +template <> struct TupleHash<FIELD_TYPE_UNSIGNED> {
> + static uint32_t hash(struct tuple *tuple, struct key_def *key_def)
> {
> assert(!key_def->is_multikey);
> - const char *field = tuple_field_by_part(tuple,
> - key_def->parts,
> - MULTIKEY_NONE);
> + const char *field = tuple_field_by_part(tuple, key_def->parts,
> + MULTIKEY_NONE);
> uint64_t val = mp_decode_uint(&field);
> if (likely(val <= UINT32_MAX))
> return val;
> - return ((uint32_t)((val)>>33^(val)^(val)<<11));
> + return ((uint32_t)((val) >> 33 ^ (val) ^ (val) << 11));
> }
> };
>
> -}; /* namespace { */
> +}; // namespace
152. We never use // comments.
>
> -#define HASHER(...) \
> - { KeyHash<__VA_ARGS__>::hash, TupleHash<__VA_ARGS__>::hash, \
> - { __VA_ARGS__, UINT32_MAX } },
> +#define HASHER(...) \
> + { KeyHash<__VA_ARGS__>::hash, \
> + TupleHash<__VA_ARGS__>::hash, \
> + { __VA_ARGS__, UINT32_MAX } },
>
> struct hasher_signature {
> key_hash_t kf;
> @@ -199,20 +189,32 @@ struct hasher_signature {
> * field1 type, field2 type, ...
> */
> static const hasher_signature hash_arr[] = {
> - HASHER(FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_UNSIGNED, FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_UNSIGNED, FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING , FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_STRING , FIELD_TYPE_UNSIGNED)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING , FIELD_TYPE_STRING)
> - HASHER(FIELD_TYPE_STRING , FIELD_TYPE_STRING , FIELD_TYPE_STRING)
> + HASHER(FIELD_TYPE_UNSIGNED) HASHER(FIELD_TYPE_STRING) HASHER(
> + FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_UNSIGNED) HASHER(FIELD_TYPE_STRING,
> + FIELD_TYPE_UNSIGNED)
> + HASHER(FIELD_TYPE_UNSIGNED, FIELD_TYPE_STRING) HASHER(
> + FIELD_TYPE_STRING,
> + FIELD_TYPE_STRING) HASHER(FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_UNSIGNED)
> + HASHER(FIELD_TYPE_STRING, FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_UNSIGNED) HASHER(FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_STRING,
> + FIELD_TYPE_UNSIGNED)
> + HASHER(FIELD_TYPE_STRING, FIELD_TYPE_STRING,
> + FIELD_TYPE_UNSIGNED) HASHER(FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_STRING)
> + HASHER(FIELD_TYPE_STRING,
> + FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_STRING)
> + HASHER(FIELD_TYPE_UNSIGNED,
> + FIELD_TYPE_STRING,
> + FIELD_TYPE_STRING)
> + HASHER(FIELD_TYPE_STRING,
> + FIELD_TYPE_STRING,
> + FIELD_TYPE_STRING)
153. Shit. Absolute pure shit. Another evidence you didn't open the patch. You
didn't even try.
> };
>
> #undef HASHER
> @@ -304,8 +308,8 @@ tuple_hash_field(uint32_t *ph1, uint32_t *pcarry, const char **field,
> */
> double iptr;
> double val = mp_typeof(**field) == MP_FLOAT ?
> - mp_decode_float(field) :
> - mp_decode_double(field);
> + mp_decode_float(field) :
> + mp_decode_double(field);
154. Ugly and random.
> if (!isfinite(val) || modf(val, &iptr) != 0 ||
> val < -exp2(63) || val >= exp2(64)) {
> size = *field - f;
> diff --git a/src/box/txn.c b/src/box/txn.c
> index eb725aaa9..6a2ebe731 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -159,7 +159,8 @@ txn_rollback_to_svp(struct txn *txn, struct stailq_entry *svp)
> struct stailq rollback;
> stailq_cut_tail(&txn->stmts, svp, &rollback);
> stailq_reverse(&rollback);
> - stailq_foreach_entry(stmt, &rollback, next) {
> + stailq_foreach_entry(stmt, &rollback, next)
> + {
155. This is a for loop and is supposed to be formatted like a
for loop.
> txn_rollback_one_stmt(txn, stmt);
> if (stmt->row != NULL && stmt->row->replica_id == 0) {
> assert(txn->n_new_rows > 0);
> @@ -237,8 +237,7 @@ txn_free(struct txn *txn)
> rlist_del(&txn->in_read_view_txs);
>
> struct txn_stmt *stmt;
> - stailq_foreach_entry(stmt, &txn->stmts, next)
> - txn_stmt_destroy(stmt);
> + stailq_foreach_entry(stmt, &txn->stmts, next) txn_stmt_destroy(stmt);
156. Ditto. We never write loop body on the same line as the loop expression.
>
> /* Truncate region up to struct txn size. */
> region_truncate(&txn->region, sizeof(struct txn));
> @@ -622,7 +622,8 @@ txn_journal_entry_new(struct txn *txn)
> struct xrow_header **local_row = req->rows + txn->n_applier_rows;
> bool is_sync = false;
>
> - stailq_foreach_entry(stmt, &txn->stmts, next) {
> + stailq_foreach_entry(stmt, &txn->stmts, next)
> + {
157. Ditto.
> if (stmt->has_triggers) {
> txn_init_triggers(txn);
> @@ -1160,8 +1160,10 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp)
> diag_set(ClientError, ER_NO_TRANSACTION);
> return -1;
> }
> - struct txn_stmt *stmt = svp->stmt == NULL ? NULL :
> - stailq_entry(svp->stmt, struct txn_stmt, next);
> + struct txn_stmt *stmt = svp->stmt == NULL ?
> + NULL :
> + stailq_entry(svp->stmt, struct txn_stmt,
> + next);
158. Ugly and random.
> if (stmt != NULL && stmt->space == NULL && stmt->row == NULL) {
> /*
> * The statement at which this savepoint was
> @@ -1188,10 +1190,12 @@ txn_savepoint_release(struct txn_savepoint *svp)
> struct txn *txn = in_txn();
> assert(txn != NULL);
> /* Make sure that savepoint hasn't been released yet. */
> - struct txn_stmt *stmt = svp->stmt == NULL ? NULL :
> - stailq_entry(svp->stmt, struct txn_stmt, next);
> + struct txn_stmt *stmt = svp->stmt == NULL ?
> + NULL :
> + stailq_entry(svp->stmt, struct txn_stmt,
> + next);
159. Ugly and random.
> assert(stmt == NULL || (stmt->space != NULL && stmt->row != NULL));
> - (void) stmt;
> + (void)stmt;
> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
> index 908a17fcc..c27f0b228 100644
> --- a/src/box/txn_limbo.c
> +++ b/src/box/txn_limbo.c
> @@ -165,8 +165,7 @@ txn_limbo_assign_local_lsn(struct txn_limbo *limbo,
> struct vclock_iterator iter;
> vclock_iterator_init(&iter, &limbo->vclock);
> int ack_count = 0;
> - vclock_foreach(&iter, vc)
> - ack_count += vc.lsn >= lsn;
> + vclock_foreach(&iter, vc) ack_count += vc.lsn >= lsn;
160. Ditto.
> assert(ack_count >= entry->ack_count);
> entry->ack_count = ack_count;
> }
> @@ -234,8 +233,8 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
>
> txn_limbo_write_rollback(limbo, entry->lsn);
> struct txn_limbo_entry *e, *tmp;
> - rlist_foreach_entry_safe_reverse(e, &limbo->queue,
> - in_queue, tmp) {
> + rlist_foreach_entry_safe_reverse(e, &limbo->queue, in_queue, tmp)
> + {
161. Ditto.
> e->txn->signature = TXN_SIGNATURE_QUORUM_TIMEOUT;
> txn_limbo_abort(limbo, e);
> txn_clear_flag(e->txn, TXN_WAIT_SYNC);
> @@ -405,7 +403,8 @@ txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn)
> assert(limbo->instance_id != REPLICA_ID_NIL);
> struct txn_limbo_entry *e, *tmp;
> struct txn_limbo_entry *last_rollback = NULL;
> - rlist_foreach_entry_reverse(e, &limbo->queue, in_queue) {
> + rlist_foreach_entry_reverse(e, &limbo->queue, in_queue)
> + {
162. Ditto.
> if (!txn_has_flag(e->txn, TXN_WAIT_ACK))
> continue;
> if (e->lsn < lsn)
> @@ -414,7 +413,8 @@ txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn)
> }
> if (last_rollback == NULL)
> return;
> - rlist_foreach_entry_safe_reverse(e, &limbo->queue, in_queue, tmp) {
> + rlist_foreach_entry_safe_reverse(e, &limbo->queue, in_queue, tmp)
> + {
163. Ditto.
> txn_limbo_abort(limbo, e);
> txn_clear_flag(e->txn, TXN_WAIT_SYNC);
> txn_clear_flag(e->txn, TXN_WAIT_ACK);
> diff --git a/src/box/user.cc b/src/box/user.cc
> index 5042fb16f..2e30cd7ce 100644
> --- a/src/box/user.cc
> +++ b/src/box/user.cc
> @@ -430,12 +417,12 @@ auth_token_get(void)
> */
> tnt_raise(LoggedError, ER_USER_MAX, BOX_USER_MAX);
> }
> - /*
> + /*
> * find-first-set returns bit index starting from 1,
> * or 0 if no bit is set. Rebase the index to offset 0.
> */
164. Another proof of the formatter being useless. It should have
re-aligned the whole comment, not just the first line.
> bit_no--;
> - tokens[min_token_idx] ^= ((umap_int_t) 1) << bit_no;
> + tokens[min_token_idx] ^= ((umap_int_t)1) << bit_no;
> int auth_token = min_token_idx * UMAP_INT_BITS + bit_no;
> assert(auth_token < UINT8_MAX);
> return auth_token;
> diff --git a/src/box/vclock.c b/src/box/vclock.c
> index 90ae27591..00e45631b 100644
> --- a/src/box/vclock.c
> +++ b/src/box/vclock.c
> @@ -60,9 +60,10 @@ vclock_snprint(char *buf, int size, const struct vclock *vclock)
> const char *sep = "";
> struct vclock_iterator it;
> vclock_iterator_init(&it, vclock);
> - vclock_foreach(&it, replica) {
> - SNPRINT(total, snprintf, buf, size, "%s%u: %lld",
> - sep, (unsigned)replica.id, (long long)replica.lsn);
> + vclock_foreach(&it, replica)
> + {
165. This is a for loop and is supposed to be formatted like a
for loop.
> + SNPRINT(total, snprintf, buf, size, "%s%u: %lld", sep,
> + (unsigned)replica.id, (long long)replica.lsn);
> sep = ", ";
> }
> diff --git a/src/box/vinyl.c b/src/box/vinyl.c
> index 57e283991..d57dbd38e 100644
> --- a/src/box/vinyl.c
> +++ b/src/box/vinyl.c
> @@ -1306,8 +1309,8 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
> struct vy_entry full_entry;
> if (pk_entry.stmt != NULL) {
> vy_stmt_foreach_entry(full_entry, pk_entry.stmt, lsm->cmp_def) {
> - if (vy_entry_compare(full_entry, entry,
> - lsm->cmp_def) == 0) {
> + if (vy_entry_compare(full_entry, entry, lsm->cmp_def) ==
> + 0) {
166. Ugly.
> match = true;
> break;
> }
> @@ -1821,8 +1819,8 @@ vy_perform_update(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
> assert(stmt->old_tuple != NULL);
> assert(stmt->new_tuple != NULL);
>
> - if (vy_check_is_unique(env, tx, space, stmt->new_tuple,
> - column_mask) != 0)
> + if (vy_check_is_unique(env, tx, space, stmt->new_tuple, column_mask) !=
> + 0)
167. Ugly.
> return -1;
> @@ -3760,8 +3749,9 @@ vinyl_index_get(struct index *index, const char *key,
> struct vy_lsm *lsm = vy_lsm(index);
> struct vy_env *env = vy_env(index->engine);
> struct vy_tx *tx = in_txn() ? in_txn()->engine_tx : NULL;
> - const struct vy_read_view **rv = (tx != NULL ? vy_tx_read_view(tx) :
> - &env->xm->p_global_read_view);
> + const struct vy_read_view **rv = (tx != NULL ?
> + vy_tx_read_view(tx) :
> + &env->xm->p_global_read_view);
168. Ugly.
>
> if (tx != NULL && tx->state == VINYL_TX_ABORT) {
> diag_set(ClientError, ER_TRANSACTION_CONFLICT);
> @@ -4109,7 +4100,8 @@ vy_build_recover(struct vy_env *env, struct vy_lsm *lsm, struct vy_lsm *pk)
> size_t mem_used_before, mem_used_after;
>
> mem_used_before = lsregion_used(&env->mem_env.allocator);
> - rlist_foreach_entry_reverse(mem, &pk->sealed, in_sealed) {
> + rlist_foreach_entry_reverse(mem, &pk->sealed, in_sealed)
> + {
169. This is a for loop and is supposed to be formatted like a
for loop.
> rc = vy_build_recover_mem(lsm, pk, mem);
> if (rc != 0)
> break;
> @@ -4561,7 +4551,7 @@ static const struct index_vtab vinyl_index_vtab = {
> /* .update_def = */ vinyl_index_update_def,
> /* .depends_on_pk = */ vinyl_index_depends_on_pk,
> /* .def_change_requires_rebuild = */
> - vinyl_index_def_change_requires_rebuild,
> + vinyl_index_def_change_requires_rebuild,
> /* .size = */ vinyl_index_size,
> /* .bsize = */ vinyl_index_bsize,
> /* .min = */ generic_index_min,
> @@ -4572,7 +4562,7 @@ static const struct index_vtab vinyl_index_vtab = {
> /* .replace = */ generic_index_replace,
> /* .create_iterator = */ vinyl_index_create_iterator,
> /* .create_snapshot_iterator = */
> - vinyl_index_create_snapshot_iterator,
> + vinyl_index_create_snapshot_iterator,
170. In this and in the previous hunk +1 tab was done intentionally, to
show that it is a part of the previous line with the corresponding comment.
Now you ruined it.
> /* .stat = */ vinyl_index_stat,
> /* .compact = */ vinyl_index_compact,
> /* .reset_stat = */ vinyl_index_reset_stat,
> diff --git a/src/box/vy_cache.c b/src/box/vy_cache.c
> index 7007d0e35..af2c5b126 100644
> --- a/src/box/vy_cache.c
> +++ b/src/box/vy_cache.c
> @@ -335,7 +330,7 @@ vy_cache_add(struct vy_cache *cache, struct vy_entry curr,
>
> /* The flag that must be set in the inserted chain node */
> uint32_t flag = direction > 0 ? VY_CACHE_LEFT_LINKED :
> - VY_CACHE_RIGHT_LINKED;
> + VY_CACHE_RIGHT_LINKED;
171. Ugly and random.
>
> #ifndef NDEBUG
> @@ -658,16 +654,17 @@ vy_cache_iterator_seek(struct vy_cache_iterator *itr, struct vy_entry last)
> if (last.stmt != NULL) {
> key = last;
> iterator_type = iterator_direction(itr->iterator_type) > 0 ?
> - ITER_GT : ITER_LT;
> + ITER_GT :
> + ITER_LT;
172. Ugly and random.
> }
>
> bool exact = false;
> if (!vy_stmt_is_empty_key(key.stmt)) {
> - itr->curr_pos = iterator_type == ITER_EQ ||
> - iterator_type == ITER_GE ||
> - iterator_type == ITER_LT ?
> - vy_cache_tree_lower_bound(tree, key, &exact) :
> - vy_cache_tree_upper_bound(tree, key, &exact);
> + itr->curr_pos =
> + iterator_type == ITER_EQ || iterator_type == ITER_GE ||
> + iterator_type == ITER_LT ?
> + vy_cache_tree_lower_bound(tree, key, &exact) :
> + vy_cache_tree_upper_bound(tree, key, &exact);
> } else if (iterator_type == ITER_LE) {
> itr->curr_pos = vy_cache_tree_invalid_iterator();
> } else {
> @@ -734,7 +731,9 @@ vy_cache_iterator_skip(struct vy_cache_iterator *itr, struct vy_entry last,
> if (itr->search_started &&
> (itr->curr.stmt == NULL || last.stmt == NULL ||
> iterator_direction(itr->iterator_type) *
> - vy_entry_compare(itr->curr, last, itr->cache->cmp_def) > 0))
> + vy_entry_compare(itr->curr, last,
> + itr->cache->cmp_def) >
> + 0))
173. Ugly and random.
> return 0;
> diff --git a/src/box/vy_log.c b/src/box/vy_log.c
> index 06b25962a..879623416 100644
> --- a/src/box/vy_log.c
> +++ b/src/box/vy_log.c
> @@ -814,8 +805,7 @@ vy_log_tx_flush(struct vy_log_tx *tx)
>
> int tx_size = 0;
> struct vy_log_record *record;
> - stailq_foreach_entry(record, &tx->records, in_tx)
> - tx_size++;
> + stailq_foreach_entry(record, &tx->records, in_tx) tx_size++;
174. This is a for loop and is supposed to be formatted like a
for loop.
>
> size_t used = region_used(&fiber()->gc);
>
> @@ -835,7 +825,8 @@ vy_log_tx_flush(struct vy_log_tx *tx)
> * Encode buffered records.
> */
> int i = 0;
> - stailq_foreach_entry(record, &tx->records, in_tx) {
> + stailq_foreach_entry(record, &tx->records, in_tx)
> + {
175. Ditto.
> if (record->gc_lsn == VY_LOG_GC_LSN_CURRENT)
> record->gc_lsn = vy_log_signature();
> assert(i < tx_size);
> @@ -1113,7 +1104,8 @@ vy_log_end_recovery(void)
> * recovery - we will need them for garbage collection.
> */
> struct vy_log_tx *tx;
> - stailq_foreach_entry(tx, &vy_log.pending_tx, in_pending) {
> + stailq_foreach_entry(tx, &vy_log.pending_tx, in_pending)
> + {
176. Ditto.
> struct vy_log_record *record;
> stailq_foreach_entry(record, &tx->records, in_tx)
> vy_recovery_process_record(vy_log.recovery, record);
> @@ -1413,7 +1405,8 @@ vy_recovery_alloc_key_parts(const struct key_part_def *key_parts,
> uint32_t new_parts_sz = sizeof(*key_parts) * key_part_count;
> for (uint32_t i = 0; i < key_part_count; i++) {
> new_parts_sz += key_parts[i].path != NULL ?
> - strlen(key_parts[i].path) + 1 : 0;
> + strlen(key_parts[i].path) + 1 :
> + 0;
177. Ugly and random.
> }
> @@ -2548,7 +2541,8 @@ vy_log_append_lsm(struct xlog *xlog, struct vy_lsm_recovery_info *lsm)
> * while we are supposed to return slices in chronological
> * order, so use reverse iterator.
> */
> - rlist_foreach_entry_reverse(slice, &range->slices, in_range) {
> + rlist_foreach_entry_reverse(slice, &range->slices, in_range)
> + {
178. This is a for loop and is supposed to be formatted like a
for loop.
> vy_log_record_init(&record);
> record.type = VY_LOG_INSERT_SLICE;
> record.range_id = range->id;
> diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
> index 1f67bea52..ce77f20a6 100644
> --- a/src/box/vy_lsm.c
> +++ b/src/box/vy_lsm.c
> @@ -483,9 +476,10 @@ vy_lsm_recover_range(struct vy_lsm *lsm,
> * order, so use reverse iterator.
> */
> struct vy_slice_recovery_info *slice_info;
> - rlist_foreach_entry_reverse(slice_info, &range_info->slices, in_range) {
> - if (vy_lsm_recover_slice(lsm, range, slice_info,
> - run_env, force_recovery) == NULL) {
> + rlist_foreach_entry_reverse(slice_info, &range_info->slices, in_range)
> + {
179. Ditto.
> + if (vy_lsm_recover_slice(lsm, range, slice_info, run_env,
> + force_recovery) == NULL) {
> vy_range_delete(range);
> range = NULL;
> goto out;
> @@ -659,12 +654,12 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery,
> (prev->end.stmt == NULL || range->begin.stmt == NULL ||
> (cmp = vy_entry_compare(prev->end, range->begin,
> lsm->cmp_def)) != 0)) {
> - const char *errmsg = cmp > 0 ?
> - "Nearby ranges %lld and %lld overlap" :
> - "Keys between ranges %lld and %lld not spanned";
> + const char *errmsg =
> + cmp > 0 ?
> + "Nearby ranges %lld and %lld overlap" :
> + "Keys between ranges %lld and %lld not spanned";
180. Ugly as shit.
> diag_set(ClientError, ER_INVALID_VYLOG_FILE,
> - tt_sprintf(errmsg,
> - (long long)prev->id,
> + tt_sprintf(errmsg, (long long)prev->id,
> (long long)range->id));
> return -1;
> }
> @@ -690,8 +685,11 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery,
> int64_t
> vy_lsm_generation(struct vy_lsm *lsm)
> {
> - struct vy_mem *oldest = rlist_empty(&lsm->sealed) ? lsm->mem :
> - rlist_last_entry(&lsm->sealed, struct vy_mem, in_sealed);
> + struct vy_mem *oldest = rlist_empty(&lsm->sealed) ?
> + lsm->mem :
> + rlist_last_entry(&lsm->sealed,
> + struct vy_mem,
> + in_sealed);
181. Ugly and random.
> return oldest->generation;
> }
>
> @@ -722,8 +720,10 @@ vy_lsm_range_size(struct vy_lsm *lsm)
> * create four times more than that for better smoothing.
> */
> int range_count = 4 * vy_lsm_dumps_per_compaction(lsm);
> - int64_t range_size = range_count == 0 ? 0 :
> - lsm->stat.disk.last_level_count.bytes / range_count;
> + int64_t range_size = range_count == 0 ?
> + 0 :
> + lsm->stat.disk.last_level_count.bytes /
> + range_count;
182. Ugly and random.
> range_size = MAX(range_size, VY_MIN_RANGE_SIZE);
> range_size = MIN(range_size, VY_MAX_RANGE_SIZE);
> return range_size;
> @@ -1161,7 +1163,8 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range)
> * so to preserve the order of the slices list, we have
> * to iterate backward.
> */
> - rlist_foreach_entry_reverse(slice, &range->slices, in_range) {
> + rlist_foreach_entry_reverse(slice, &range->slices, in_range)
> + {
183. This is a for loop and is supposed to be formatted like a
for loop.
> if (vy_slice_cut(slice, vy_log_next_id(), part->begin,
> part->end, lsm->cmp_def,
> &new_slice) != 0)
> diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
> index 98027e784..5ceb15418 100644
> --- a/src/box/vy_mem.c
> +++ b/src/box/vy_mem.c
> @@ -390,7 +389,8 @@ vy_mem_iterator_seek(struct vy_mem_iterator *itr, struct vy_entry last)
> if (last.stmt != NULL) {
> key = last;
> iterator_type = iterator_direction(itr->iterator_type) > 0 ?
> - ITER_GT : ITER_LT;
> + ITER_GT :
> + ITER_LT;
184. Ugly and random.
> }
>
> bool exact = false;
> @@ -577,7 +577,9 @@ vy_mem_iterator_skip(struct vy_mem_iterator *itr, struct vy_entry last,
> if (itr->search_started &&
> (itr->curr.stmt == NULL || last.stmt == NULL ||
> iterator_direction(itr->iterator_type) *
> - vy_entry_compare(itr->curr, last, itr->mem->cmp_def) > 0))
> + vy_entry_compare(itr->curr, last,
> + itr->mem->cmp_def) >
> + 0))
185. Ugly.
> return 0;
> diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
> index 409796910..b35e90c05 100644
> --- a/src/box/vy_read_iterator.c
> +++ b/src/box/vy_read_iterator.c
> @@ -648,7 +646,8 @@ vy_read_iterator_open(struct vy_read_iterator *itr, struct vy_lsm *lsm,
> * in this case.
> */
> itr->iterator_type = iterator_direction(iterator_type) > 0 ?
> - ITER_GE : ITER_LE;
> + ITER_GE :
> + ITER_LE;
186. Ugly and random.
> }
> diff --git a/src/box/vy_regulator.c b/src/box/vy_regulator.c
> index 8ec7e25d6..2083016dd 100644
> --- a/src/box/vy_regulator.c
> +++ b/src/box/vy_regulator.c
> @@ -209,17 +209,17 @@ vy_regulator_create(struct vy_regulator *regulator, struct vy_quota *quota,
> enum { KB = 1024, MB = KB * KB };
> static int64_t dump_bandwidth_buckets[] = {
> 100 * KB, 200 * KB, 300 * KB, 400 * KB, 500 * KB, 600 * KB,
> - 700 * KB, 800 * KB, 900 * KB, 1 * MB, 2 * MB, 3 * MB,
> - 4 * MB, 5 * MB, 6 * MB, 7 * MB, 8 * MB, 9 * MB,
> - 10 * MB, 15 * MB, 20 * MB, 25 * MB, 30 * MB, 35 * MB,
> - 40 * MB, 45 * MB, 50 * MB, 55 * MB, 60 * MB, 65 * MB,
> - 70 * MB, 75 * MB, 80 * MB, 85 * MB, 90 * MB, 95 * MB,
> + 700 * KB, 800 * KB, 900 * KB, 1 * MB, 2 * MB, 3 * MB,
> + 4 * MB, 5 * MB, 6 * MB, 7 * MB, 8 * MB, 9 * MB,
> + 10 * MB, 15 * MB, 20 * MB, 25 * MB, 30 * MB, 35 * MB,
> + 40 * MB, 45 * MB, 50 * MB, 55 * MB, 60 * MB, 65 * MB,
> + 70 * MB, 75 * MB, 80 * MB, 85 * MB, 90 * MB, 95 * MB,
> 100 * MB, 200 * MB, 300 * MB, 400 * MB, 500 * MB, 600 * MB,
> 700 * MB, 800 * MB, 900 * MB,
187. It was much better before.
> };
> diff --git a/src/box/vy_run.c b/src/box/vy_run.c
> index b9822dc3e..ea9d10f19 100644
> --- a/src/box/vy_run.c
> +++ b/src/box/vy_run.c
> @@ -65,10 +63,10 @@ static const uint64_t vy_run_info_key_map = (1 << VY_RUN_INFO_MIN_KEY) |
> #define XLOG_META_TYPE_INDEX "INDEX"
>
> const char *vy_file_suffix[] = {
> - "index", /* VY_FILE_INDEX */
> - "index" inprogress_suffix, /* VY_FILE_INDEX_INPROGRESS */
> - "run", /* VY_FILE_RUN */
> - "run" inprogress_suffix, /* VY_FILE_RUN_INPROGRESS */
> + "index", /* VY_FILE_INDEX */
> + "index" inprogress_suffix, /* VY_FILE_INDEX_INPROGRESS */
> + "run", /* VY_FILE_RUN */
> + "run" inprogress_suffix, /* VY_FILE_RUN_INPROGRESS */
188. Extra indentation was used to avoid unnecessary changes
when new longer values are added. So as not to re-aign all the other
lines. Now you ruined it.
> };
>
> /* sync run and index files very 16 MB */
> @@ -1372,7 +1367,8 @@ vy_run_iterator_seek(struct vy_run_iterator *itr, struct vy_entry last,
> if (iterator_type == ITER_EQ)
> check_eq = true;
> iterator_type = iterator_direction(iterator_type) > 0 ?
> - ITER_GT : ITER_LT;
> + ITER_GT :
> + ITER_LT;
189. Ugly and random.
> key = last;
> }
> @@ -1610,7 +1604,8 @@ vy_run_iterator_skip(struct vy_run_iterator *itr, struct vy_entry last,
> if (itr->search_started &&
> (itr->curr.stmt == NULL || last.stmt == NULL ||
> iterator_direction(itr->iterator_type) *
> - vy_entry_compare(itr->curr, last, itr->cmp_def) > 0))
> + vy_entry_compare(itr->curr, last, itr->cmp_def) >
> + 0))
190. Ugly.
> return 0;
> @@ -1786,11 +1781,12 @@ vy_run_dump_stmt(struct vy_entry entry, struct xlog *data_xlog,
> bool is_primary)
> {
> struct xrow_header xrow;
> - int rc = (is_primary ?
> - vy_stmt_encode_primary(entry.stmt, key_def, 0, &xrow) :
> - vy_stmt_encode_secondary(entry.stmt, key_def,
> - vy_entry_multikey_idx(entry, key_def),
> - &xrow));
> + int rc =
> + (is_primary ?
> + vy_stmt_encode_primary(entry.stmt, key_def, 0, &xrow) :
> + vy_stmt_encode_secondary(
> + entry.stmt, key_def,
> + vy_entry_multikey_idx(entry, key_def), &xrow));
191. Ugly and random. This is broken on each line of this hunk.
> if (rc != 0)
> @@ -1938,15 +1930,11 @@ vy_page_info_encode(const struct vy_page_info *page_info,
> static size_t
> vy_stmt_stat_sizeof(const struct vy_stmt_stat *stat)
> {
> - return mp_sizeof_map(4) +
> - mp_sizeof_uint(IPROTO_INSERT) +
> - mp_sizeof_uint(IPROTO_REPLACE) +
> - mp_sizeof_uint(IPROTO_DELETE) +
> - mp_sizeof_uint(IPROTO_UPSERT) +
> - mp_sizeof_uint(stat->inserts) +
> - mp_sizeof_uint(stat->replaces) +
> - mp_sizeof_uint(stat->deletes) +
> - mp_sizeof_uint(stat->upserts);
> + return mp_sizeof_map(4) + mp_sizeof_uint(IPROTO_INSERT) +
> + mp_sizeof_uint(IPROTO_REPLACE) + mp_sizeof_uint(IPROTO_DELETE) +
> + mp_sizeof_uint(IPROTO_UPSERT) + mp_sizeof_uint(stat->inserts) +
> + mp_sizeof_uint(stat->replaces) + mp_sizeof_uint(stat->deletes) +
> + mp_sizeof_uint(stat->upserts);
192. It was better before. Easier to read.
> }
> @@ -2181,12 +2166,13 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
> if (run->info.page_count >= writer->page_info_capacity &&
> vy_run_alloc_page_info(run, &writer->page_info_capacity) != 0)
> return -1;
> - const char *key = vy_stmt_is_key(first_entry.stmt) ?
> - tuple_data(first_entry.stmt) :
> - tuple_extract_key(first_entry.stmt, writer->cmp_def,
> - vy_entry_multikey_idx(first_entry,
> - writer->cmp_def),
> - NULL);
> + const char *key =
> + vy_stmt_is_key(first_entry.stmt) ?
> + tuple_data(first_entry.stmt) :
> + tuple_extract_key(first_entry.stmt, writer->cmp_def,
> + vy_entry_multikey_idx(
> + first_entry, writer->cmp_def),
> + NULL);
193. Ugly and random.
> if (key == NULL)
> return -1;
> if (run->info.page_count == 0) {
> @@ -2337,11 +2323,13 @@ vy_run_writer_commit(struct vy_run_writer *writer)
>
> assert(writer->last.stmt != NULL);
> const char *key = vy_stmt_is_key(writer->last.stmt) ?
> - tuple_data(writer->last.stmt) :
> - tuple_extract_key(writer->last.stmt, writer->cmp_def,
> - vy_entry_multikey_idx(writer->last,
> - writer->cmp_def),
> - NULL);
> + tuple_data(writer->last.stmt) :
> + tuple_extract_key(writer->last.stmt,
> + writer->cmp_def,
> + vy_entry_multikey_idx(
> + writer->last,
> + writer->cmp_def),
> + NULL);
194. Ugly and random.
> if (key == NULL)
> @@ -2445,16 +2433,17 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
> if (tuple == NULL)
> goto close_err;
> if (bloom_builder != NULL) {
> - struct vy_entry entry = {tuple, HINT_NONE};
> + struct vy_entry entry = { tuple, HINT_NONE };
> if (vy_bloom_builder_add(bloom_builder, entry,
> key_def) != 0) {
> tuple_unref(tuple);
> goto close_err;
> }
> }
> - key = vy_stmt_is_key(tuple) ? tuple_data(tuple) :
> - tuple_extract_key(tuple, cmp_def,
> - MULTIKEY_NONE, NULL);
> + key = vy_stmt_is_key(tuple) ?
> + tuple_data(tuple) :
> + tuple_extract_key(tuple, cmp_def,
> + MULTIKEY_NONE, NULL);
195. Ugly and random. Sqlite code looks better.
> diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
> index b641dd9b8..234cc69ed 100644
> --- a/src/box/vy_scheduler.c
> +++ b/src/box/vy_scheduler.c
> @@ -60,15 +60,19 @@
> #include "trivia/util.h"
>
> /* Min and max values for vy_scheduler::timeout. */
> -#define VY_SCHEDULER_TIMEOUT_MIN 1
> -#define VY_SCHEDULER_TIMEOUT_MAX 60
> +#define VY_SCHEDULER_TIMEOUT_MIN 1
> +#define VY_SCHEDULER_TIMEOUT_MAX 60
>
> static int vy_worker_f(va_list);
> static int vy_scheduler_f(va_list);
> -static void vy_task_execute_f(struct cmsg *);
> -static void vy_task_complete_f(struct cmsg *);
> -static void vy_deferred_delete_batch_process_f(struct cmsg *);
> -static void vy_deferred_delete_batch_free_f(struct cmsg *);
> +static void
> +vy_task_execute_f(struct cmsg *);
> +static void
> +vy_task_complete_f(struct cmsg *);
> +static void
> +vy_deferred_delete_batch_process_f(struct cmsg *);
> +static void
> +vy_deferred_delete_batch_free_f(struct cmsg *);
196. The formatting is inconsistent. Functions vy_worker_f and vy_scheduler_f
stayed not formatted. It seems the formatter does not know what to do with
va_list argument.
> static const struct cmsg_hop vy_task_execute_route[] = {
> { vy_task_execute_f, NULL },
> @@ -865,8 +871,8 @@ vy_deferred_delete_process_one(struct space *deferred_delete_space,
> return -1;
>
> struct tuple *unused;
> - if (space_execute_dml(deferred_delete_space, txn,
> - &request, &unused) != 0) {
> + if (space_execute_dml(deferred_delete_space, txn, &request, &unused) !=
> + 0) {
197. Ugly.
> txn_rollback_stmt(txn);
> return -1;
> }
> @@ -1648,8 +1654,9 @@ vy_task_compaction_new(struct vy_scheduler *scheduler, struct vy_worker *worker,
> bool is_last_level = (range->compaction_priority == range->slice_count);
> wi = vy_write_iterator_new(task->cmp_def, lsm->index_id == 0,
> is_last_level, scheduler->read_views,
> - lsm->index_id > 0 ? NULL :
> - &task->deferred_delete_handler);
> + lsm->index_id > 0 ?
> + NULL :
> + &task->deferred_delete_handler);
198. Ugly and random.
> if (wi == NULL)
> goto err_wi;
>
> @@ -1657,8 +1664,8 @@ vy_task_compaction_new(struct vy_scheduler *scheduler, struct vy_worker *worker,
> int32_t dump_count = 0;
> int n = range->compaction_priority;
> rlist_foreach_entry(slice, &range->slices, in_range) {
> - if (vy_write_iterator_new_slice(wi, slice,
> - lsm->disk_format) != 0)
> + if (vy_write_iterator_new_slice(wi, slice, lsm->disk_format) !=
> + 0)
199. Ugly.
> goto err_wi_sub;
> new_run->dump_lsn = MAX(new_run->dump_lsn,
> slice->run->dump_lsn);
> @@ -1908,7 +1916,7 @@ retry:
> }
> if (*ptask == NULL)
> goto retry; /* LSM tree dropped or range split/coalesced */
> - return 0; /* new task */
> + return 0; /* new task */
200. It seems the formatter tries to align /* */ comments
on adjacent lines. In this case it is wrong.
> no_task:
> if (worker != NULL)
> vy_worker_pool_put(worker);
> @@ -1992,7 +1998,8 @@ vy_scheduler_f(va_list va)
>
> /* Complete and delete all processed tasks. */
> stailq_foreach_entry_safe(task, next, &processed_tasks,
> - in_processed) {
> + in_processed)
> + {
201. This is a for loop and is supposed to be formatted like a
for loop.
> if (vy_task_complete(task) != 0)
> tasks_failed++;
> else
> @@ -2035,7 +2042,7 @@ vy_scheduler_f(va_list va)
>
> fiber_reschedule();
> continue;
> -error:
> + error:
202. Labels are visible everywhere inside a function, and should not
have indentation.
> /* Abort pending checkpoint. */
> fiber_cond_signal(&scheduler->dump_cond);
> /*
> diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
> index 92e0aa1c5..a62f1c9c0 100644
> --- a/src/box/vy_stmt.c
> +++ b/src/box/vy_stmt.c
> @@ -652,9 +656,9 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
> switch (type) {
> case IPROTO_DELETE:
> extracted = vy_stmt_is_key(value) ?
> - tuple_data_range(value, &size) :
> - tuple_extract_key(value, key_def,
> - MULTIKEY_NONE, &size);
> + tuple_data_range(value, &size) :
> + tuple_extract_key(value, key_def,
> + MULTIKEY_NONE, &size);
203. Ugly and random.
> if (extracted == NULL)
> return -1;
> request.key = extracted;
> @@ -697,9 +701,9 @@ vy_stmt_encode_secondary(struct tuple *value, struct key_def *cmp_def,
> request.type = type;
> uint32_t size;
> const char *extracted = vy_stmt_is_key(value) ?
> - tuple_data_range(value, &size) :
> - tuple_extract_key(value, cmp_def,
> - multikey_idx, &size);
> + tuple_data_range(value, &size) :
> + tuple_extract_key(value, cmp_def,
> + multikey_idx, &size);
204. Ugly and random.
> if (extracted == NULL)
> return -1;
> diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
> index 24c7eaad7..e4bf3024a 100644
> --- a/src/box/vy_stmt.h
> +++ b/src/box/vy_stmt.h
> @@ -767,15 +764,18 @@ vy_entry_compare_with_raw_key(struct vy_entry entry,
> *
> * entry.stmt is set to src_stmt on each iteration.
> */
> -#define vy_stmt_foreach_entry(entry, src_stmt, key_def) \
> - for (uint32_t multikey_idx = 0, \
> - multikey_count = !(key_def)->is_multikey ? 1 : \
> - tuple_multikey_count((src_stmt), (key_def)); \
> - multikey_idx < multikey_count && \
> - (((entry).stmt = (src_stmt)), \
> - ((entry).hint = !(key_def)->is_multikey ? \
> - vy_stmt_hint((src_stmt), (key_def)) : \
> - multikey_idx), true); \
> +#define vy_stmt_foreach_entry(entry, src_stmt, key_def) \
> + for (uint32_t multikey_idx = 0, \
> + multikey_count = !(key_def)->is_multikey ? \
> + 1 : \
> + tuple_multikey_count( \
> + (src_stmt), (key_def)); \
> + multikey_idx < multikey_count && \
> + (((entry).stmt = (src_stmt)), \
> + ((entry).hint = !(key_def)->is_multikey ? \
> + vy_stmt_hint((src_stmt), (key_def)) : \
> + multikey_idx), \
> + true); \
205. Extra indentation of \ was used to avoid unnecessary changes
when new longer lines are added. So as not to re-aign all the other
lines. Now you ruined it.
Now it not only will change when a new line is added, but also the
entire block now is misaligned in some lines. Mostly due to the
formatter not being able to format operator ?.
> ++multikey_idx)
>
> diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
> index ff63cd7a1..d3c3fd6b5 100644
> --- a/src/box/vy_tx.c
> +++ b/src/box/vy_tx.c
> @@ -343,8 +341,7 @@ vy_tx_destroy(struct vy_tx *tx)
> vy_tx_manager_destroy_read_view(tx->xm, tx->read_view);
>
> struct txv *v, *tmp;
> - stailq_foreach_entry_safe(v, tmp, &tx->log, next_in_log)
> - txv_delete(v);
> + stailq_foreach_entry_safe(v, tmp, &tx->log, next_in_log) txv_delete(v);
206. This is a for loop and is supposed to be formatted like a
for loop.
>
> vy_tx_read_set_iter(&tx->read_set, NULL, vy_tx_read_set_free_cb, NULL);
> rlist_del_entry(tx, in_writers);
> @@ -704,7 +701,8 @@ vy_tx_prepare(struct vy_tx *tx)
> /* repsert - REPLACE/UPSERT */
> struct tuple *delete = NULL, *repsert = NULL;
> MAYBE_UNUSED uint32_t current_space_id = 0;
> - stailq_foreach_entry(v, &tx->log, next_in_log) {
> + stailq_foreach_entry(v, &tx->log, next_in_log)
> + {
207. Ditto.
> struct vy_lsm *lsm = v->lsm;
> if (lsm->index_id == 0) {
> @@ -780,8 +779,8 @@ vy_tx_prepare(struct vy_tx *tx)
>
> /* In secondary indexes only REPLACE/DELETE can be written. */
> vy_stmt_set_lsn(v->entry.stmt, MAX_LSN + tx->psn);
> - struct tuple **region_stmt =
> - (type == IPROTO_DELETE) ? &delete : &repsert;
> + struct tuple **region_stmt = (type == IPROTO_DELETE) ? &delete :
> + &repsert;
208. Ugly and random.
> if (vy_tx_write(lsm, v->mem, v->entry, region_stmt) != 0)
> return -1;
> v->region_stmt = *region_stmt;
> @@ -809,7 +808,8 @@ vy_tx_commit(struct vy_tx *tx, int64_t lsn)
>
> /* Fix LSNs of the records and commit changes. */
> struct txv *v;
> - stailq_foreach_entry(v, &tx->log, next_in_log) {
> + stailq_foreach_entry(v, &tx->log, next_in_log)
> + {
209. This is a for loop and is supposed to be formatted like a
for loop.
> if (v->region_stmt != NULL) {
> struct vy_entry entry;
> entry.stmt = v->region_stmt;
> @@ -858,7 +858,8 @@ vy_tx_rollback_after_prepare(struct vy_tx *tx)
> xm->last_prepared_tx = NULL;
>
> struct txv *v;
> - stailq_foreach_entry(v, &tx->log, next_in_log) {
> + stailq_foreach_entry(v, &tx->log, next_in_log)
> + {
210. Ditto.
> if (v->region_stmt != NULL) {
> struct vy_entry entry;
> entry.stmt = v->region_stmt;
> @@ -919,7 +919,8 @@ vy_tx_rollback_statement(struct vy_tx *tx, void *svp)
> /* Rollback statements in LIFO order. */
> stailq_reverse(&tail);
> struct txv *v, *tmp;
> - stailq_foreach_entry_safe(v, tmp, &tail, next_in_log) {
> + stailq_foreach_entry_safe(v, tmp, &tail, next_in_log)
> + {
211. Ditto.
> write_set_remove(&tx->write_set, v);
> if (v->overwritten != NULL) {
> /* Restore overwritten statement. */
> @@ -1006,7 +1006,8 @@ vy_tx_track(struct vy_tx *tx, struct vy_lsm *lsm,
> }
> struct vy_read_interval *next_interval;
> stailq_foreach_entry_safe(interval, next_interval, &merge,
> - in_merge) {
> + in_merge)
> + {
212. Ditto.
> vy_tx_read_set_remove(&tx->read_set, interval);
> vy_lsm_read_set_remove(&lsm->read_set, interval);
> vy_read_interval_delete(interval);
> @@ -1210,7 +1209,8 @@ vy_txw_iterator_seek(struct vy_txw_iterator *itr, struct vy_entry last)
> if (last.stmt != NULL) {
> key = last;
> iterator_type = iterator_direction(iterator_type) > 0 ?
> - ITER_GT : ITER_LT;
> + ITER_GT :
> + ITER_LT;
213. Ugly and random.
> }
>
> struct vy_lsm *lsm = itr->lsm;
> @@ -1305,8 +1308,9 @@ vy_txw_iterator_skip(struct vy_txw_iterator *itr, struct vy_entry last,
> if (itr->search_started &&
> (itr->curr_txv == NULL || last.stmt == NULL ||
> iterator_direction(itr->iterator_type) *
> - vy_entry_compare(itr->curr_txv->entry, last,
> - itr->lsm->cmp_def) > 0))
> + vy_entry_compare(itr->curr_txv->entry, last,
> + itr->lsm->cmp_def) >
> + 0))
214. Ugly.
> return 0;
> diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
> index 78a52ae5c..924698afd 100644
> --- a/src/box/vy_write_iterator.c
> +++ b/src/box/vy_write_iterator.c
> @@ -787,11 +785,10 @@ vy_write_iterator_build_history(struct vy_write_iterator *stream,
> vy_stmt_type(src->entry.stmt) == IPROTO_DELETE) {
> current_rv_i++;
> current_rv_lsn = merge_until_lsn;
> - merge_until_lsn =
> - vy_write_iterator_get_vlsn(stream,
> - current_rv_i + 1);
> + merge_until_lsn = vy_write_iterator_get_vlsn(
> + stream, current_rv_i + 1);
> }
> -next_lsn:
> + next_lsn:
> rc = vy_write_iterator_merge_step(stream);
215. Labels are visible everywhere inside a function, and should not
have indentation.
> if (rc != 0)
> break;
> @@ -1034,7 +1030,8 @@ vy_write_iterator_build_read_views(struct vy_write_iterator *stream, int *count)
> for (; rv >= &stream->read_views[0]; --rv) {
> if (rv->history == NULL)
> continue;
> - if (vy_read_view_merge(stream, prev, rv, is_first_insert) != 0) {
> + if (vy_read_view_merge(stream, prev, rv, is_first_insert) !=
> + 0) {
216. Ugly.
> rc = -1;
> goto cleanup;
> }
> diff --git a/src/box/wal.c b/src/box/wal.c
> index 84abaa7b2..99b40feb0 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -417,10 +415,9 @@ wal_writer_create(struct wal_writer *writer, enum wal_mode wal_mode,
> writer->wal_max_size = wal_max_size;
>
> journal_create(&writer->base,
> - wal_mode == WAL_NONE ?
> - wal_write_none_async : wal_write_async,
> - wal_mode == WAL_NONE ?
> - wal_write_none : wal_write);
> + wal_mode == WAL_NONE ? wal_write_none_async :
> + wal_write_async,
> + wal_mode == WAL_NONE ? wal_write_none : wal_write);
217. Ugly.
> @@ -938,8 +933,8 @@ out:
> };
> struct tx_notify_gc_msg *msg = malloc(sizeof(*msg));
> if (msg != NULL) {
> - if (xdir_first_vclock(&writer->wal_dir,
> - &msg->vclock) < 0)
> + if (xdir_first_vclock(&writer->wal_dir, &msg->vclock) <
> + 0)
218. Ugly.
> vclock_copy(&msg->vclock, &writer->vclock);
> cmsg_init(&msg->base, route);
> @@ -991,19 +986,22 @@ wal_assign_lsn(struct vclock *vclock_diff, struct vclock *base,
> (*row)->tsn = tsn == 0 ? (*start)->lsn : tsn;
> (*row)->is_commit = row == end - 1;
> } else {
> - int64_t diff = (*row)->lsn - vclock_get(base, (*row)->replica_id);
> - if (diff <= vclock_get(vclock_diff,
> - (*row)->replica_id)) {
> + int64_t diff = (*row)->lsn -
> + vclock_get(base, (*row)->replica_id);
> + if (diff <=
> + vclock_get(vclock_diff, (*row)->replica_id)) {
> say_crit("Attempt to write a broken LSN to WAL:"
> " replica id: %d, confirmed lsn: %d,"
> - " new lsn %d", (*row)->replica_id,
> + " new lsn %d",
> + (*row)->replica_id,
> vclock_get(base, (*row)->replica_id) +
> - vclock_get(vclock_diff,
> - (*row)->replica_id),
> - (*row)->lsn);
> + vclock_get(vclock_diff,
> + (*row)->replica_id),
> + (*row)->lsn);
219. This extra indentation for multiline arith looks worse in the code.
When you look at it not in diff.
> assert(false);
> } else {
> - vclock_follow(vclock_diff, (*row)->replica_id, diff);
> + vclock_follow(vclock_diff, (*row)->replica_id,
> + diff);
> }
> }
> }
> @@ -1090,9 +1088,10 @@ wal_write_to_disk(struct cmsg *msg)
> int rc;
> struct journal_entry *entry;
> struct stailq_entry *last_committed = NULL;
> - stailq_foreach_entry(entry, &wal_msg->commit, fifo) {
> - wal_assign_lsn(&vclock_diff, &writer->vclock,
> - entry->rows, entry->rows + entry->n_rows);
> + stailq_foreach_entry(entry, &wal_msg->commit, fifo)
> + {
220. This is a for loop and is supposed to be formatted like a
for loop.
> + wal_assign_lsn(&vclock_diff, &writer->vclock, entry->rows,
> + entry->rows + entry->n_rows);
> entry->res = vclock_sum(&vclock_diff) +
> vclock_sum(&writer->vclock);
> rc = xlog_write_entry(l, entry);
> @@ -1160,8 +1159,7 @@ done:
>
> if (!stailq_empty(&rollback)) {
> /* Update status of the successfully committed requests. */
> - stailq_foreach_entry(entry, &rollback, fifo)
> - entry->res = -1;
> + stailq_foreach_entry(entry, &rollback, fifo) entry->res = -1;
221. Ditto.
> /* Rollback unprocessed requests */
> stailq_concat(&wal_msg->rollback, &rollback);
> @@ -1199,11 +1197,11 @@ wal_writer_f(va_list ap)
> */
> if (writer->wal_mode != WAL_NONE &&
> (!xlog_is_open(&writer->current_wal) ||
> - vclock_compare(&writer->vclock,
> - &writer->current_wal.meta.vclock) > 0)) {
> + vclock_compare(&writer->vclock, &writer->current_wal.meta.vclock) >
> + 0)) {
> struct xlog l;
> - if (xdir_create_xlog(&writer->wal_dir, &l,
> - &writer->vclock) == 0)
> + if (xdir_create_xlog(&writer->wal_dir, &l, &writer->vclock) ==
> + 0)
222. Ugly.
> xlog_close(&l, false);
> else
> diag_log();
> @@ -1226,13 +1224,11 @@ wal_writer_f(va_list ap)
> static int
> wal_write_async(struct journal *journal, struct journal_entry *entry)
> {
> - struct wal_writer *writer = (struct wal_writer *) journal;
> + struct wal_writer *writer = (struct wal_writer *)journal;
>
> - ERROR_INJECT(ERRINJ_WAL_IO, {
> - goto fail;
> - });
> + ERROR_INJECT(ERRINJ_WAL_IO, { goto fail; });
223. Injections are like 'if' statements, and should have similar formatting.
So the previous version was correct.
>
> - if (! stailq_empty(&writer->rollback)) {
> + if (!stailq_empty(&writer->rollback)) {
> @@ -1489,10 +1481,10 @@ wal_set_watcher(struct wal_watcher *watcher, const char *name,
> watcher->pending_events = 0;
>
> assert(lengthof(watcher->route) == 2);
> - watcher->route[0] = (struct cmsg_hop)
> - { wal_watcher_notify_perform, &watcher->wal_pipe };
> - watcher->route[1] = (struct cmsg_hop)
> - { wal_watcher_notify_complete, NULL };
> + watcher->route[0] = (struct cmsg_hop){ wal_watcher_notify_perform,
> + &watcher->wal_pipe };
> + watcher->route[1] = (struct cmsg_hop){ wal_watcher_notify_complete,
> + NULL };
224. Ugly.
> cbus_pair("wal", name, &watcher->wal_pipe, &watcher->watcher_pipe,
> wal_watcher_attach, watcher, process_cb);
> }
> diff --git a/src/box/xlog.c b/src/box/xlog.c
> index 974f460be..eeb482e98 100644
> --- a/src/box/xlog.c
> +++ b/src/box/xlog.c
> @@ -67,9 +67,12 @@
> */
> typedef uint32_t log_magic_t;
>
> -static const log_magic_t row_marker = mp_bswap_u32(0xd5ba0bab); /* host byte order */
> -static const log_magic_t zrow_marker = mp_bswap_u32(0xd5ba0bba); /* host byte order */
> -static const log_magic_t eof_marker = mp_bswap_u32(0xd510aded); /* host byte order */
> +static const log_magic_t row_marker = mp_bswap_u32(
> + 0xd5ba0bab); /* host byte order */
> +static const log_magic_t zrow_marker = mp_bswap_u32(
> + 0xd5ba0bba); /* host byte order */
> +static const log_magic_t eof_marker = mp_bswap_u32(
> + 0xd510aded); /* host byte order */
225. It would be better to move the comments on a separate line, and
leave the assignments in single lines.
>
> enum {
> @@ -154,9 +156,7 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
> int total = 0;
> SNPRINT(total, snprintf, buf, size,
> "%s\n"
> - "%s\n"
> - VERSION_KEY ": %s\n"
> - INSTANCE_UUID_KEY ": %s\n",
> + "%s\n" VERSION_KEY ": %s\n" INSTANCE_UUID_KEY ": %s\n",
226. The string was intentionally split into multiple lines by \n, to
represent how will it look in the file. Now it is broken.
> meta->filetype, v13, PACKAGE_VERSION,
> tt_uuid_str(&meta->instance_uuid));
> if (vclock_is_set(&meta->vclock)) {
> @@ -286,14 +285,17 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
> * Instance: <uuid>
> */
> if (val_end - val != UUID_STR_LEN) {
> - diag_set(XlogError, "can't parse instance UUID");
> + diag_set(XlogError,
> + "can't parse instance UUID");
> return -1;
> }
> char uuid[UUID_STR_LEN + 1];
> memcpy(uuid, val, UUID_STR_LEN);
> uuid[UUID_STR_LEN] = '\0';
> - if (tt_uuid_from_string(uuid, &meta->instance_uuid) != 0) {
> - diag_set(XlogError, "can't parse instance UUID");
> + if (tt_uuid_from_string(uuid, &meta->instance_uuid) !=
> + 0) {
227. Ugly.
> + diag_set(XlogError,
> + "can't parse instance UUID");
> return -1;
> }
> @@ -927,7 +927,7 @@ xlog_open(struct xlog *xlog, const char *name, const struct xlog_opts *opts)
> goto err_read;
> }
> if (rc != sizeof(magic) || load_u32(magic) != eof_marker) {
> -no_eof:
> + no_eof:
228. Labels are visible everywhere inside a function, and should not
have indentation.
> xlog->offset = fio_lseek(xlog->fd, 0, SEEK_END);
> if (xlog->offset < 0) {
> diag_set(SystemError, "failed to seek file '%s'",
> @@ -991,12 +991,12 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
> prev_vclock = vclockset_last(&dir->index);
>
> struct xlog_meta meta;
> - xlog_meta_create(&meta, dir->filetype, dir->instance_uuid,
> - vclock, prev_vclock);
> + xlog_meta_create(&meta, dir->filetype, dir->instance_uuid, vclock,
> + prev_vclock);
>
> const char *filename = xdir_format_filename(dir, signature, NONE);
> - if (xlog_create(xlog, filename, dir->open_wflags, &meta,
> - &dir->opts) != 0)
> + if (xlog_create(xlog, filename, dir->open_wflags, &meta, &dir->opts) !=
> + 0)
229. Ugly.
> return -1;
>
> /* Rename xlog file */
> @@ -1245,16 +1244,17 @@ xlog_tx_write(struct xlog *log)
> log->offset += written;
> log->rows += log->tx_rows;
> log->tx_rows = 0;
> - if ((log->opts.sync_interval && log->offset >=
> - (off_t)(log->synced_size + log->opts.sync_interval)) ||
> - (log->opts.rate_limit && log->offset >=
> - (off_t)(log->synced_size + log->opts.rate_limit))) {
> + if ((log->opts.sync_interval &&
> + log->offset >=
> + (off_t)(log->synced_size + log->opts.sync_interval)) ||
> + (log->opts.rate_limit &&
> + log->offset >= (off_t)(log->synced_size + log->opts.rate_limit))) {
230. Ugly.
> off_t sync_from = SYNC_ROUND_DOWN(log->synced_size);
> - size_t sync_len = SYNC_ROUND_UP(log->offset) -
> - sync_from;
> + size_t sync_len = SYNC_ROUND_UP(log->offset) - sync_from;
> if (log->opts.rate_limit > 0) {
> double throttle_time;
> - throttle_time = (double)sync_len / log->opts.rate_limit -
> + throttle_time = (double)sync_len /
> + log->opts.rate_limit -
> (ev_monotonic_time() - log->sync_time);
231. Ugly.
> if (throttle_time > 0)
> ev_sleep(throttle_time);
> @@ -1263,8 +1263,8 @@ xlog_tx_write(struct xlog *log)
> #ifdef HAVE_SYNC_FILE_RANGE
> sync_file_range(log->fd, sync_from, sync_len,
> SYNC_FILE_RANGE_WAIT_BEFORE |
> - SYNC_FILE_RANGE_WRITE |
> - SYNC_FILE_RANGE_WAIT_AFTER);
> + SYNC_FILE_RANGE_WRITE |
> + SYNC_FILE_RANGE_WAIT_AFTER);
232. Ugly.
> #else
> @@ -1970,13 +1969,14 @@ xlog_cursor_openfd(struct xlog_cursor *i, int fd, const char *name)
> rc = xlog_cursor_ensure(i, XLOG_META_LEN_MAX);
> if (rc == -1)
> goto error;
> - rc = xlog_meta_parse(&i->meta,
> - (const char **)&i->rbuf.rpos,
> + rc = xlog_meta_parse(&i->meta, (const char **)&i->rbuf.rpos,
> (const char *)i->rbuf.wpos);
> if (rc == -1)
> goto error;
> if (rc > 0) {
> - diag_set(XlogError, "Unexpected end of file, run with 'force_recovery = true'");
> + diag_set(
> + XlogError,
> + "Unexpected end of file, run with 'force_recovery = true'");
233. Ugly.
> goto error;
> }
> snprintf(i->name, sizeof(i->name), "%s", name);
> @@ -2061,8 +2060,8 @@ xlog_cursor_close(struct xlog_cursor *i, bool reuse_fd)
> if (i->state == XLOG_CURSOR_TX)
> xlog_tx_cursor_destroy(&i->tx_cursor);
> ZSTD_freeDStream(i->zdctx);
> - i->state = (i->state == XLOG_CURSOR_EOF ?
> - XLOG_CURSOR_EOF_CLOSED : XLOG_CURSOR_CLOSED);
> + i->state = (i->state == XLOG_CURSOR_EOF ? XLOG_CURSOR_EOF_CLOSED :
> + XLOG_CURSOR_CLOSED);
234. Ugly.
> /*
> * Do not trash the cursor object since the caller might
> * still want to access its state and/or meta information.
> diff --git a/src/box/xlog.h b/src/box/xlog.h
> index 5b1f42ce1..cb7c2d329 100644
> --- a/src/box/xlog.h
> +++ b/src/box/xlog.h
> @@ -94,9 +94,9 @@ extern const struct xlog_opts xlog_opts_default;
> * but an xlog object sees only those files which match its type.
> */
> enum xdir_type {
> - SNAP, /* memtx snapshot */
> - XLOG, /* write ahead log */
> - VYLOG, /* vinyl metadata log */
> + SNAP, /* memtx snapshot */
> + XLOG, /* write ahead log */
> + VYLOG, /* vinyl metadata log */
235. Extra indentation was used to avoid unnecessary changes
when new longer values are added. So as not to re-aign all the other
lines. Now you ruined it.
> };
>
> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index da5c6ffae..def90e277 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -46,15 +46,16 @@
> #include "mpstream/mpstream.h"
>
> static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < 0x7f &&
> - IPROTO_SQL_INFO < 0x7f, "encoded IPROTO_BODY keys must fit into "\
> + IPROTO_SQL_INFO < 0x7f,
> + "encoded IPROTO_BODY keys must fit into "
> "one byte");
236. Ugly.
> @@ -136,10 +141,11 @@ xrow_header_decode(struct xrow_header *header, const char **pos,
> {
> memset(header, 0, sizeof(struct xrow_header));
> const char *tmp = *pos;
> - const char * const start = *pos;
> + const char *const start = *pos;
> if (mp_check(&tmp, end) != 0) {
> -error:
> - xrow_on_decode_err(start, end, ER_INVALID_MSGPACK, "packet header");
> + error:
237. Labels are visible everywhere inside a function, and should not
have indentation.
> + xrow_on_decode_err(start, end, ER_INVALID_MSGPACK,
> + "packet header");
> return -1;
> @@ -339,18 +347,18 @@ xrow_encode_uuid(char *pos, const struct tt_uuid *in)
>
> /* m_ - msgpack meta, k_ - key, v_ - value */
> struct PACKED iproto_header_bin {
> - uint8_t m_len; /* MP_UINT32 */
> - uint32_t v_len; /* length */
> - uint8_t m_header; /* MP_MAP */
> - uint8_t k_code; /* IPROTO_REQUEST_TYPE */
> - uint8_t m_code; /* MP_UINT32 */
> - uint32_t v_code; /* response status */
> - uint8_t k_sync; /* IPROTO_SYNC */
> - uint8_t m_sync; /* MP_UINT64 */
> - uint64_t v_sync; /* sync */
> - uint8_t k_schema_version; /* IPROTO_SCHEMA_VERSION */
> - uint8_t m_schema_version; /* MP_UINT32 */
> - uint32_t v_schema_version; /* schema_version */
> + uint8_t m_len; /* MP_UINT32 */
> + uint32_t v_len; /* length */
> + uint8_t m_header; /* MP_MAP */
> + uint8_t k_code; /* IPROTO_REQUEST_TYPE */
> + uint8_t m_code; /* MP_UINT32 */
> + uint32_t v_code; /* response status */
> + uint8_t k_sync; /* IPROTO_SYNC */
> + uint8_t m_sync; /* MP_UINT64 */
> + uint64_t v_sync; /* sync */
> + uint8_t k_schema_version; /* IPROTO_SCHEMA_VERSION */
> + uint8_t m_schema_version; /* MP_UINT32 */
> + uint32_t v_schema_version; /* schema_version */
238. Extra indentation was used to avoid unnecessary changes
when new longer values are added. So as not to re-aign all the other
lines. Now you ruined it.
> };
>
> static_assert(sizeof(struct iproto_header_bin) == IPROTO_HEADER_LEN,
> @@ -378,18 +386,18 @@ iproto_header_encode(char *out, uint32_t type, uint64_t sync,
> }
>
> struct PACKED iproto_body_bin {
> - uint8_t m_body; /* MP_MAP */
> - uint8_t k_data; /* IPROTO_DATA or errors */
> - uint8_t m_data; /* MP_STR or MP_ARRAY */
> - uint32_t v_data_len; /* string length of array size */
> + uint8_t m_body; /* MP_MAP */
> + uint8_t k_data; /* IPROTO_DATA or errors */
> + uint8_t m_data; /* MP_STR or MP_ARRAY */
> + uint32_t v_data_len; /* string length of array size */
239. Ditto.
> };
>
> static_assert(sizeof(struct iproto_body_bin) + IPROTO_HEADER_LEN ==
> - IPROTO_SELECT_HEADER_LEN, "size of the prepared select");
> + IPROTO_SELECT_HEADER_LEN,
240. Ugly.
> + "size of the prepared select");
>
> -static const struct iproto_body_bin iproto_body_bin = {
> - 0x81, IPROTO_DATA, 0xdd, 0
> -};
> +static const struct iproto_body_bin iproto_body_bin = { 0x81, IPROTO_DATA, 0xdd,
> + 0 };
241. Ugly.
> @@ -603,18 +611,19 @@ int
> xrow_decode_sql(const struct xrow_header *row, struct sql_request *request)
> {
> if (row->bodycnt == 0) {
> - diag_set(ClientError, ER_INVALID_MSGPACK, "missing request body");
> + diag_set(ClientError, ER_INVALID_MSGPACK,
> + "missing request body");
> return 1;
> }
> assert(row->bodycnt == 1);
> - const char *data = (const char *) row->body[0].iov_base;
> + const char *data = (const char *)row->body[0].iov_base;
> const char *end = data + row->body[0].iov_len;
> assert((end - data) > 0);
>
> if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
> -error:
> - xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
> - "packet body");
> + error:
242. Labels are visible everywhere inside a function, and should not
have indentation.
> + xrow_on_decode_err(row->body[0].iov_base, end,
> + ER_INVALID_MSGPACK, "packet body");
> return -1;
> }
> @@ -695,20 +705,20 @@ xrow_decode_dml(struct xrow_header *row, struct request *request,
> goto done;
>
> assert(row->bodycnt == 1);
> - const char *data = start = (const char *) row->body[0].iov_base;
> + const char *data = start = (const char *)row->body[0].iov_base;
> end = data + row->body[0].iov_len;
> assert((end - data) > 0);
>
> if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
> -error:
> - xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
> - "packet body");
> + error:
243. Ditto.
> + xrow_on_decode_err(row->body[0].iov_base, end,
> + ER_INVALID_MSGPACK, "packet body");
> return -1;
> }
> @@ -1103,14 +1110,14 @@ xrow_decode_call(const struct xrow_header *row, struct call_request *request)
> }
>
> assert(row->bodycnt == 1);
> - const char *data = (const char *) row->body[0].iov_base;
> + const char *data = (const char *)row->body[0].iov_base;
> const char *end = data + row->body[0].iov_len;
> assert((end - data) > 0);
>
> if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
> -error:
> - xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
> - "packet body");
> + error:
244. Ditto.
> + xrow_on_decode_err(row->body[0].iov_base, end,
> + ER_INVALID_MSGPACK, "packet body");
> return -1;
> }
> @@ -1184,14 +1192,14 @@ xrow_decode_auth(const struct xrow_header *row, struct auth_request *request)
> }
>
> assert(row->bodycnt == 1);
> - const char *data = (const char *) row->body[0].iov_base;
> + const char *data = (const char *)row->body[0].iov_base;
> const char *end = data + row->body[0].iov_len;
> assert((end - data) > 0);
>
> if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
> -error:
> - xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
> - "packet body");
> + error:
245. Ditto.
> + xrow_on_decode_err(row->body[0].iov_base, end,
> + ER_INVALID_MSGPACK, "packet body");
> return -1;
> @@ -1402,8 +1413,8 @@ xrow_decode_ballot(struct xrow_header *row, struct ballot *ballot)
> ballot->is_anon = mp_decode_bool(&data);
> break;
> case IPROTO_BALLOT_VCLOCK:
> - if (mp_decode_vclock_ignore0(&data,
> - &ballot->vclock) != 0)
> + if (mp_decode_vclock_ignore0(&data, &ballot->vclock) !=
> + 0)
246. Ugly.
> goto err;
> break;
> case IPROTO_BALLOT_GC_VCLOCK:
> @@ -1583,7 +1593,9 @@ xrow_decode_subscribe(struct xrow_header *row, struct tt_uuid *replicaset_uuid,
> if (id_filter == NULL)
> goto skip;
> if (mp_typeof(*d) != MP_ARRAY) {
> -id_filter_decode_err: xrow_on_decode_err(data, end, ER_INVALID_MSGPACK,
> + id_filter_decode_err:
247. Labels are visible everywhere inside a function, and should not
have indentation.
> + xrow_on_decode_err(data, end,
> + ER_INVALID_MSGPACK,
> "invalid ID_FILTER");
> return -1;
> }
> @@ -1726,17 +1739,19 @@ greeting_decode(const char *greetingbuf, struct greeting *greeting)
> int h = IPROTO_GREETING_SIZE / 2;
> const char *pos = greetingbuf + strlen("Tarantool ");
> const char *end = greetingbuf + h;
> - for (; pos < end && *pos == ' '; ++pos); /* skip spaces */
> + for (; pos < end && *pos == ' '; ++pos)
> + ; /* skip spaces */
248. Ugly.
>
> /* Extract a version string - a string until ' ' */
> char version[20];
> - const char *vend = (const char *) memchr(pos, ' ', end - pos);
> + const char *vend = (const char *)memchr(pos, ' ', end - pos);
> if (vend == NULL || (size_t)(vend - pos) >= sizeof(version))
> return -1;
> memcpy(version, pos, vend - pos);
> version[vend - pos] = '\0';
> pos = vend + 1;
> - for (; pos < end && *pos == ' '; ++pos); /* skip spaces */
> + for (; pos < end && *pos == ' '; ++pos)
> + ; /* skip spaces */
249. Ugly.
>
> /* Parse a version string - 1.6.6-83-gc6b2129 or 1.6.7 */
> @@ -1759,10 +1774,12 @@ greeting_decode(const char *greetingbuf, struct greeting *greeting)
> if (greeting->version_id >= version_id(1, 6, 7)) {
> if (*(pos++) != ' ')
> return -1;
> - for (; pos < end && *pos == ' '; ++pos); /* spaces */
> + for (; pos < end && *pos == ' '; ++pos)
> + ; /* spaces */
250. Ugly.
> if (end - pos < UUID_STR_LEN)
> return -1;
> - if (tt_uuid_from_strl(pos, UUID_STR_LEN, &greeting->uuid))
> + if (tt_uuid_from_strl(pos, UUID_STR_LEN,
> + &greeting->uuid))
> return -1;
> }
> diff --git a/src/box/xrow_update_array.c b/src/box/xrow_update_array.c
> index 717466be7..795e56872 100644
> --- a/src/box/xrow_update_array.c
> +++ b/src/box/xrow_update_array.c
> @@ -404,29 +406,29 @@ xrow_update_op_do_array_delete(struct xrow_update_op *op,
> return 0;
> }
>
> -#define DO_SCALAR_OP_GENERIC(op_type) \
> -int \
> -xrow_update_op_do_array_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - if (xrow_update_op_prepare_num_token(op) != 0) \
> - return -1; \
> - struct xrow_update_array_item *item = \
> - xrow_update_array_extract_item(field, op); \
> - if (item == NULL) \
> - return -1; \
> - if (!xrow_update_op_is_term(op)) { \
> - op->is_token_consumed = true; \
> - return xrow_update_op_do_field_##op_type(op, &item->field); \
> - } \
> - if (item->field.type != XUPDATE_NOP) \
> - return xrow_update_err_double(op); \
> - if (xrow_update_op_do_##op_type(op, item->field.data) != 0) \
> - return -1; \
> - item->field.type = XUPDATE_SCALAR; \
> - item->field.scalar.op = op; \
> - return 0; \
> -}
> +#define DO_SCALAR_OP_GENERIC(op_type) \
> + int xrow_update_op_do_array_##op_type(struct xrow_update_op *op, \
> + struct xrow_update_field *field) \
> + { \
> + if (xrow_update_op_prepare_num_token(op) != 0) \
> + return -1; \
> + struct xrow_update_array_item *item = \
> + xrow_update_array_extract_item(field, op); \
> + if (item == NULL) \
> + return -1; \
> + if (!xrow_update_op_is_term(op)) { \
> + op->is_token_consumed = true; \
> + return xrow_update_op_do_field_##op_type( \
> + op, &item->field); \
> + } \
> + if (item->field.type != XUPDATE_NOP) \
> + return xrow_update_err_double(op); \
> + if (xrow_update_op_do_##op_type(op, item->field.data) != 0) \
> + return -1; \
> + item->field.type = XUPDATE_SCALAR; \
> + item->field.scalar.op = op; \
> + return 0; \
> + }
251. Return type should be one separate line. And we don't
add +1 tab inside macros.
> DO_SCALAR_OP_GENERIC(arith)
>
> diff --git a/src/box/xrow_update_bar.c b/src/box/xrow_update_bar.c
> index 796f340cd..d245efc0f 100644
> --- a/src/box/xrow_update_bar.c
> +++ b/src/box/xrow_update_bar.c
> @@ -306,19 +304,18 @@ xrow_update_op_do_nop_delete(struct xrow_update_op *op,
> return xrow_update_bar_finish(field);
> }
>
> -#define DO_NOP_OP_GENERIC(op_type) \
> -int \
> -xrow_update_op_do_nop_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - assert(field->type == XUPDATE_NOP); \
> - int key_len_or_index; \
> - if (xrow_update_bar_locate(op, field, &key_len_or_index) != 0) \
> - return -1; \
> - if (xrow_update_op_do_##op_type(op, field->bar.point) != 0) \
> - return -1; \
> - return xrow_update_bar_finish(field); \
> -}
> +#define DO_NOP_OP_GENERIC(op_type) \
> + int xrow_update_op_do_nop_##op_type(struct xrow_update_op *op, \
> + struct xrow_update_field *field) \
> + { \
> + assert(field->type == XUPDATE_NOP); \
> + int key_len_or_index; \
> + if (xrow_update_bar_locate(op, field, &key_len_or_index) != 0) \
> + return -1; \
> + if (xrow_update_op_do_##op_type(op, field->bar.point) != 0) \
> + return -1; \
> + return xrow_update_bar_finish(field); \
> + }
252. Ditto.
>
> DO_NOP_OP_GENERIC(arith)
>
> @@ -328,17 +325,16 @@ DO_NOP_OP_GENERIC(splice)
>
> #undef DO_NOP_OP_GENERIC
>
> -#define DO_BAR_OP_GENERIC(op_type) \
> -int \
> -xrow_update_op_do_bar_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - assert(field->type == XUPDATE_BAR); \
> - field = xrow_update_route_branch(field, op); \
> - if (field == NULL) \
> - return -1; \
> - return xrow_update_op_do_field_##op_type(op, field); \
> -}
> +#define DO_BAR_OP_GENERIC(op_type) \
> + int xrow_update_op_do_bar_##op_type(struct xrow_update_op *op, \
> + struct xrow_update_field *field) \
> + { \
> + assert(field->type == XUPDATE_BAR); \
> + field = xrow_update_route_branch(field, op); \
> + if (field == NULL) \
> + return -1; \
> + return xrow_update_op_do_field_##op_type(op, field); \
> + }
253. Ditto.
>
> DO_BAR_OP_GENERIC(insert)
> diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
> index 1095eceda..d05cf9444 100644
> --- a/src/box/xrow_update_field.c
> +++ b/src/box/xrow_update_field.c
> @@ -555,7 +556,8 @@ xrow_update_op_store_arith(struct xrow_update_op *op,
> if (this_node != NULL) {
> enum field_type type =
> json_tree_entry(this_node, struct tuple_field,
> - token)->type;
> + token)
> + ->type;
254. Ugly and unnecessary.
> if (type == FIELD_TYPE_DOUBLE) {
> out = mp_encode_double(out, arg->flt);
> break;
> @@ -744,9 +748,9 @@ xrow_update_op_decode(struct xrow_update_op *op, int op_num, int index_base,
> const char *path = mp_decode_str(expr, &len);
> uint32_t field_no, hash = field_name_hash(path, len);
> json_lexer_create(&op->lexer, path, len, TUPLE_INDEX_BASE);
> - if (tuple_fieldno_by_name(dict, path, len, hash,
> - &field_no) == 0) {
> - op->field_no = (int32_t) field_no;
> + if (tuple_fieldno_by_name(dict, path, len, hash, &field_no) ==
> + 0) {
255. Ugly.
> + op->field_no = (int32_t)field_no;
> op->lexer.offset = len;
> break;
> diff --git a/src/box/xrow_update_field.h b/src/box/xrow_update_field.h
> index 193df5882..3d5c39bd5 100644
> --- a/src/box/xrow_update_field.h
> +++ b/src/box/xrow_update_field.h
> @@ -482,39 +479,31 @@ xrow_update_field_store(struct xrow_update_field *field,
> * etc. Each complex type has basic operations of the same
> * signature: insert, set, delete, arith, bit, splice.
> */
> -#define OP_DECL_GENERIC(type) \
> -int \
> -xrow_update_op_do_##type##_insert(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -int \
> -xrow_update_op_do_##type##_set(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -int \
> -xrow_update_op_do_##type##_delete(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -int \
> -xrow_update_op_do_##type##_arith(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -int \
> -xrow_update_op_do_##type##_bit(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -int \
> -xrow_update_op_do_##type##_splice(struct xrow_update_op *op, \
> - struct xrow_update_field *field); \
> - \
> -uint32_t \
> -xrow_update_##type##_sizeof(struct xrow_update_field *field); \
> - \
> -uint32_t \
> -xrow_update_##type##_store(struct xrow_update_field *field, \
> - struct json_tree *format_tree, \
> - struct json_token *this_node, char *out, \
> - char *out_end);
> +#define OP_DECL_GENERIC(type) \
> + int xrow_update_op_do_##type##_insert( \
> + struct xrow_update_op *op, struct xrow_update_field *field); \
> + \
> + int xrow_update_op_do_##type##_set(struct xrow_update_op *op, \
> + struct xrow_update_field *field); \
> + \
> + int xrow_update_op_do_##type##_delete( \
> + struct xrow_update_op *op, struct xrow_update_field *field); \
> + \
> + int xrow_update_op_do_##type##_arith(struct xrow_update_op *op, \
> + struct xrow_update_field *field); \
> + \
> + int xrow_update_op_do_##type##_bit(struct xrow_update_op *op, \
> + struct xrow_update_field *field); \
> + \
> + int xrow_update_op_do_##type##_splice( \
> + struct xrow_update_op *op, struct xrow_update_field *field); \
> + \
> + uint32_t xrow_update_##type##_sizeof(struct xrow_update_field *field); \
> + \
> + uint32_t xrow_update_##type##_store(struct xrow_update_field *field, \
> + struct json_tree *format_tree, \
> + struct json_token *this_node, \
> + char *out, char *out_end);
256. Return type should be one separate line. And we don't
add +1 tab inside macros.
>
> /* }}} xrow_update_field */
>
> @@ -666,27 +655,26 @@ OP_DECL_GENERIC(route)
> * fit ~10k update tree depth - incredible number, even though the
> * real limit is 4k due to limited number of operations.
> */
> -#define OP_DECL_GENERIC(op_type) \
> -static inline int \
> -xrow_update_op_do_field_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - switch (field->type) { \
> - case XUPDATE_ARRAY: \
> - return xrow_update_op_do_array_##op_type(op, field); \
> - case XUPDATE_NOP: \
> - return xrow_update_op_do_nop_##op_type(op, field); \
> - case XUPDATE_BAR: \
> - return xrow_update_op_do_bar_##op_type(op, field); \
> - case XUPDATE_ROUTE: \
> - return xrow_update_op_do_route_##op_type(op, field); \
> - case XUPDATE_MAP: \
> - return xrow_update_op_do_map_##op_type(op, field); \
> - default: \
> - unreachable(); \
> - } \
> - return 0; \
> -}
> +#define OP_DECL_GENERIC(op_type) \
> + static inline int xrow_update_op_do_field_##op_type( \
> + struct xrow_update_op *op, struct xrow_update_field *field) \
> + { \
> + switch (field->type) { \
> + case XUPDATE_ARRAY: \
> + return xrow_update_op_do_array_##op_type(op, field); \
> + case XUPDATE_NOP: \
> + return xrow_update_op_do_nop_##op_type(op, field); \
> + case XUPDATE_BAR: \
> + return xrow_update_op_do_bar_##op_type(op, field); \
> + case XUPDATE_ROUTE: \
> + return xrow_update_op_do_route_##op_type(op, field); \
> + case XUPDATE_MAP: \
> + return xrow_update_op_do_map_##op_type(op, field); \
> + default: \
> + unreachable(); \
> + } \
> + return 0; \
> + }
257. Ditto.
>
> OP_DECL_GENERIC(insert)
>
> @@ -758,15 +746,15 @@ xrow_update_err_double(const struct xrow_update_op *op)
> static inline int
> xrow_update_err_bad_json(const struct xrow_update_op *op, int pos)
> {
> - return xrow_update_err(op, tt_sprintf("invalid JSON in position %d",
> - pos));
> + return xrow_update_err(op,
> + tt_sprintf("invalid JSON in position %d", pos));
258. Ugly.
> }
>
> static inline int
> xrow_update_err_delete1(const struct xrow_update_op *op)
> {
> - return xrow_update_err(op, "can delete only 1 field from a map in a "\
> - "row");
> + return xrow_update_err(op, "can delete only 1 field from a map in a "
> + "row");
> }
>
> static inline int
> diff --git a/src/box/xrow_update_map.c b/src/box/xrow_update_map.c
> index 57fb27f18..62d52d8fa 100644
> --- a/src/box/xrow_update_map.c
> +++ b/src/box/xrow_update_map.c
> @@ -83,8 +83,7 @@ xrow_update_map_create_item(struct xrow_update_field *field,
> item->key_len = key_len;
> item->field.type = type;
> item->field.data = data;
> - item->field.size = data_size,
> - item->tail_size = tail_size;
> + item->field.size = data_size, item->tail_size = tail_size;
259. Obviously, ',' should be ';' here.
> /*
> * Each time a new item it created it is stored in the
> * head of update map item list. It helps in case the
> @@ -123,8 +122,8 @@ xrow_update_map_extract_opt_item(struct xrow_update_field *field,
> if (xrow_update_op_next_token(op) != 0)
> return -1;
> if (op->token_type != JSON_TOKEN_STR) {
> - return xrow_update_err(op, "can't update a map by not "\
> - "a string key");
> + return xrow_update_err(op, "can't update a map by not "
> + "a string key");
> }
> }
> struct stailq *items = &field->map.items;
> @@ -136,7 +135,8 @@ xrow_update_map_extract_opt_item(struct xrow_update_field *field,
> * passing this key, so it should be here for all except
> * first updates.
> */
> - stailq_foreach_entry(i, items, in_items) {
> + stailq_foreach_entry(i, items, in_items)
> + {
260. This is a for loop and is supposed to be formatted like a
for loop.
> if (i->key != NULL && i->key_len == op->key_len &&
> memcmp(i->key, op->key, i->key_len) == 0) {
> *res = i;
> @@ -149,12 +149,13 @@ xrow_update_map_extract_opt_item(struct xrow_update_field *field,
> */
> uint32_t key_len, i_tail_size;
> const char *pos, *key, *end, *tmp, *begin;
> - stailq_foreach_entry(i, items, in_items) {
> + stailq_foreach_entry(i, items, in_items)
> + {
261. Ditto.
> begin = i->field.data + i->field.size;
> pos = begin;
> end = begin + i->tail_size;
> i_tail_size = 0;
> - while(pos < end) {
> + while (pos < end) {
> if (mp_typeof(*pos) != MP_STR) {
> mp_next(&pos);
> mp_next(&pos);
> @@ -309,28 +310,28 @@ xrow_update_op_do_map_delete(struct xrow_update_op *op,
> return 0;
> }
>
> -#define DO_SCALAR_OP_GENERIC(op_type) \
> -int \
> -xrow_update_op_do_map_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - assert(field->type == XUPDATE_MAP); \
> - struct xrow_update_map_item *item = \
> - xrow_update_map_extract_item(field, op); \
> - if (item == NULL) \
> - return -1; \
> - if (!xrow_update_op_is_term(op)) { \
> - op->is_token_consumed = true; \
> - return xrow_update_op_do_field_##op_type(op, &item->field); \
> - } \
> - if (item->field.type != XUPDATE_NOP) \
> - return xrow_update_err_double(op); \
> - if (xrow_update_op_do_##op_type(op, item->field.data) != 0) \
> - return -1; \
> - item->field.type = XUPDATE_SCALAR; \
> - item->field.scalar.op = op; \
> - return 0; \
> -}
> +#define DO_SCALAR_OP_GENERIC(op_type) \
> + int xrow_update_op_do_map_##op_type(struct xrow_update_op *op, \
> + struct xrow_update_field *field) \
> + { \
> + assert(field->type == XUPDATE_MAP); \
> + struct xrow_update_map_item *item = \
> + xrow_update_map_extract_item(field, op); \
> + if (item == NULL) \
> + return -1; \
> + if (!xrow_update_op_is_term(op)) { \
> + op->is_token_consumed = true; \
> + return xrow_update_op_do_field_##op_type( \
> + op, &item->field); \
> + } \
> + if (item->field.type != XUPDATE_NOP) \
> + return xrow_update_err_double(op); \
> + if (xrow_update_op_do_##op_type(op, item->field.data) != 0) \
> + return -1; \
> + item->field.type = XUPDATE_SCALAR; \
> + item->field.scalar.op = op; \
> + return 0; \
> + }
262. Return type should be one separate line. And we don't
add +1 tab inside macros.
>
> DO_SCALAR_OP_GENERIC(arith)
> @@ -418,7 +418,8 @@ xrow_update_map_sizeof(struct xrow_update_field *field)
> assert(field->type == XUPDATE_MAP);
> uint32_t res = mp_sizeof_map(field->map.size);
> struct xrow_update_map_item *i;
> - stailq_foreach_entry(i, &field->map.items, in_items) {
> + stailq_foreach_entry(i, &field->map.items, in_items)
> + {
263. This is a for loop and is supposed to be formatted like a
for loop.
> res += i->tail_size;
> if (i->key != NULL) {
> res += mp_sizeof_str(i->key_len) +
> @@ -442,7 +443,8 @@ xrow_update_map_store(struct xrow_update_field *field,
> * others. The first cycle doesn't save unchanged tails.
> */
> if (this_node == NULL) {
> - stailq_foreach_entry(i, &field->map.items, in_items) {
> + stailq_foreach_entry(i, &field->map.items, in_items)
> + {
264. Ditto
> if (i->key != NULL) {
> out = mp_encode_str(out, i->key, i->key_len);
> out += xrow_update_field_store(&i->field, NULL,
> @@ -454,13 +456,13 @@ xrow_update_map_store(struct xrow_update_field *field,
> struct json_token token;
> token.type = JSON_TOKEN_STR;
> struct json_token *next_node;
> - stailq_foreach_entry(i, &field->map.items, in_items) {
> + stailq_foreach_entry(i, &field->map.items, in_items)
> + {
265. Ditto
> if (i->key != NULL) {
> token.str = i->key;
> token.len = i->key_len;
> next_node = json_tree_lookup(format_tree,
> - this_node,
> - &token);
> + this_node, &token);
> out = mp_encode_str(out, i->key, i->key_len);
> out += xrow_update_field_store(&i->field,
> format_tree,
> @@ -469,7 +471,8 @@ xrow_update_map_store(struct xrow_update_field *field,
> }
> }
> }
> - stailq_foreach_entry(i, &field->map.items, in_items) {
> + stailq_foreach_entry(i, &field->map.items, in_items)
> + {
266. Ditto
> memcpy(out, i->field.data + i->field.size, i->tail_size);
> out += i->tail_size;
> }
> diff --git a/src/box/xrow_update_route.c b/src/box/xrow_update_route.c
> index 0352aec63..ee23bbef9 100644
> --- a/src/box/xrow_update_route.c
> +++ b/src/box/xrow_update_route.c
> @@ -151,8 +151,8 @@ xrow_update_route_branch_map(struct xrow_update_field *next_hop,
> mp_next(&end);
> mp_next(&end);
> }
> - if (xrow_update_map_create(next_hop, parent, data, end,
> - field_count) != 0)
> + if (xrow_update_map_create(next_hop, parent, data, end, field_count) !=
> + 0)
267. Ugly.
> return -1;
> return op->meta->do_op(op, next_hop);
> @@ -346,17 +347,17 @@ xrow_update_route_next(struct xrow_update_field *field, struct xrow_update_op *o
> return xrow_update_route_branch(field, op);
> }
>
> -#define DO_SCALAR_OP_GENERIC(op_type) \
> -int \
> -xrow_update_op_do_route_##op_type(struct xrow_update_op *op, \
> - struct xrow_update_field *field) \
> -{ \
> - assert(field->type == XUPDATE_ROUTE); \
> - struct xrow_update_field *next_hop = xrow_update_route_next(field, op); \
> - if (next_hop == NULL) \
> - return -1; \
> - return xrow_update_op_do_field_##op_type(op, next_hop); \
> -}
> +#define DO_SCALAR_OP_GENERIC(op_type) \
> + int xrow_update_op_do_route_##op_type(struct xrow_update_op *op, \
> + struct xrow_update_field *field) \
> + { \
> + assert(field->type == XUPDATE_ROUTE); \
> + struct xrow_update_field *next_hop = \
> + xrow_update_route_next(field, op); \
> + if (next_hop == NULL) \
> + return -1; \
> + return xrow_update_op_do_field_##op_type(op, next_hop); \
> + }
268. Return type should be one separate line. And we don't
add +1 tab inside macros.
More information about the Tarantool-patches
mailing list