From ac60ae2cda86e2030f79f1afae881fa5c91dfd4e Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Thu, 20 Jun 2013 14:38:47 +0400
Subject: [PATCH] tuple-transform: review fixes.

Minor style edits.
---
 src/box/bitset_index.cc |  6 ++---
 src/box/box_lua.cc      | 51 +++++++++++++++++++----------------------
 src/box/space.cc        |  2 +-
 src/box/tree_index.cc   |  2 +-
 src/box/tuple.cc        |  2 +-
 src/box/tuple_update.cc |  8 +++----
 src/lib/bitset/expr.c   |  2 +-
 src/lib/bitset/index.c  | 16 ++++++-------
 src/lib/bitset/page.h   |  6 ++---
 src/memcached.cc        |  2 +-
 10 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/src/box/bitset_index.cc b/src/box/bitset_index.cc
index 1b9070e3a5..49d9ac00ae 100644
--- a/src/box/bitset_index.cc
+++ b/src/box/bitset_index.cc
@@ -93,7 +93,7 @@ bitset_index_iterator_next(struct iterator *iterator)
 BitsetIndex::BitsetIndex(struct key_def *key_def, struct space *space)
 	: Index(key_def, space)
 {
-	assert (!key_def->is_unique);
+	assert(!key_def->is_unique);
 
 	if (bitset_index_create(&index, realloc) != 0)
 		panic_syserror("bitset_index_create");
@@ -126,7 +126,7 @@ BitsetIndex::endBuild()
 void
 BitsetIndex::build(Index *pk)
 {
-	assert (!key_def->is_unique);
+	assert(!key_def->is_unique);
 
 	struct iterator *it = pk->position();
 	struct tuple *tuple;
@@ -213,7 +213,7 @@ BitsetIndex::replace(struct tuple *old_tuple, struct tuple *new_tuple,
 		if (bitset_index_contains_value(&index, value)) {
 			ret = old_tuple;
 
-			assert (old_tuple != new_tuple);
+			assert(old_tuple != new_tuple);
 			bitset_index_remove_value(&index, value);
 		}
 	}
diff --git a/src/box/box_lua.cc b/src/box/box_lua.cc
index 1fc37f4015..ea50087cfa 100644
--- a/src/box/box_lua.cc
+++ b/src/box/box_lua.cc
@@ -200,7 +200,7 @@ lbox_tuple_transform(struct lua_State *L)
 	int argc = lua_gettop(L);
 	if (argc < 3)
 		luaL_error(L, "tuple.transform(): bad arguments");
-	lua_Integer offset = lua_tointeger(L, 2);
+	lua_Integer offset = lua_tointeger(L, 2);  /* Can be negative and can be > INT_MAX */
 	lua_Integer len = lua_tointeger(L, 3);
 
 	/* validate offset and len */
@@ -216,22 +216,20 @@ lbox_tuple_transform(struct lua_State *L)
 	if (len > tuple->field_count - offset)
 		len = tuple->field_count - offset;
 
-	assert ( ( (offset < tuple->field_count) &&
-		   (len >= 0 && offset + len <= tuple->field_count) ) ||
-		 (offset == tuple->field_count && len == 0));
+	assert(offset + len <= tuple->field_count);
 
 	/*
 	 * Calculate the number of operations and length of UPDATE expression
 	 */
-	uint32_t ops_cnt = 0;
+	uint32_t op_cnt = 0;
 	size_t expr_len = 0;
-	expr_len += sizeof(uint32_t); /* ops_count */
+	expr_len += sizeof(uint32_t); /* op_count */
 	if (offset < tuple->field_count) {
-		/* Remove `len` fields */
-		ops_cnt += len;
+		/* Add an UPDATE operation for each removed field. */
+		op_cnt += len;
 		expr_len += len * sizeof(uint32_t);	/* Field */
-		expr_len += len * sizeof(uint8_t);	/* Operation */
-		expr_len += len * varint32_sizeof(0);	/* Ignored */
+		expr_len += len * sizeof(uint8_t);	/* UPDATE_OP_DELETE */
+		expr_len += len * varint32_sizeof(0);	/* Unused */
 	}
 
 	for (int i = 4; i <= argc; i++) {
@@ -253,14 +251,13 @@ lbox_tuple_transform(struct lua_State *L)
 		}
 
 		/* Insert one field */
-		ops_cnt++;
+		op_cnt++;
 		expr_len += sizeof(uint32_t);		/* Field Number */
-		expr_len += sizeof(uint8_t);		/* Operation */
+		expr_len += sizeof(uint8_t);		/* UPDATE_OP_SET */
 		expr_len += varint32_sizeof(field_len) + field_len; /* Field */
 	}
-
-	if (ops_cnt == 0) {
-		/* Nothing to do */
+	if (op_cnt == 0) {
+		/* tuple_upate() does not accept an empty operation list. */
 		lbox_pushtuple(L, tuple);
 		return 1;
 	}
@@ -269,12 +266,12 @@ lbox_tuple_transform(struct lua_State *L)
 	 * Prepare UPDATE expression
 	 */
 	char *expr = (char *) palloc(fiber->gc_pool, expr_len);
-	char *e = expr;
-	e = pack_u32(e, ops_cnt);
-	for (lua_Integer i = 0; i < len; i++) {
-		e = pack_u32(e, offset);
-		e = pack_u8(e, UPDATE_OP_DELETE);
-		e = pack_varint32(e, 0);
+	char *pos = expr;
+	pos = pack_u32(pos, op_cnt);
+	for (uint32_t i = 0; i < (uint32_t) len; i++) {
+		pos = pack_u32(pos, offset);
+		pos = pack_u8(pos, UPDATE_OP_DELETE);
+		pos = pack_varint32(pos, 0);
 	}
 
 	for (int i = argc ; i >= 4; i--) {
@@ -297,18 +294,18 @@ lbox_tuple_transform(struct lua_State *L)
 			field = luaL_checklstring(L, i, &field_len);
 			break;
 		default:
-			assert (false);
+			assert(false);
 			break;
 		}
 
-		assert (field_len <= UINT32_MAX);
+		assert(field_len <= UINT32_MAX);
 		/* Insert the field */
-		e = pack_u32(e, offset);		/* Field Number */
-		e = pack_u8(e, UPDATE_OP_INSERT);	/* Operation */
-		e = pack_lstr(e, field, field_len);	/* Field Value */
+		pos = pack_u32(pos, offset);		/* Field Number */
+		pos = pack_u8(pos, UPDATE_OP_INSERT);	/* Operation */
+		pos = pack_lstr(pos, field, field_len);	/* Field Value */
 	}
 
-	assert (e == expr + expr_len);
+	assert(pos == expr + expr_len);
 
 	/* Execute tuple_update */
 	struct tuple *new_tuple = tuple_update(tuple, expr, expr + expr_len);
diff --git a/src/box/space.cc b/src/box/space.cc
index 52f6d24de8..dc9eb92017 100644
--- a/src/box/space.cc
+++ b/src/box/space.cc
@@ -383,7 +383,7 @@ space_config()
 			enum index_type type = STR2ENUM(index_type, cfg_index->type);
 			struct key_def *key_def = &space->key_defs[j];
 			Index *index = Index::factory(type, key_def, space);
-			assert (index != NULL);
+			assert(index != NULL);
 			space->index[j] = index;
 		}
 
diff --git a/src/box/tree_index.cc b/src/box/tree_index.cc
index 7ba2fabd21..a09c405269 100644
--- a/src/box/tree_index.cc
+++ b/src/box/tree_index.cc
@@ -304,7 +304,7 @@ key_is_linear(struct key_def *key_def)
 static void
 fold_with_sparse_parts(struct key_def *key_def, struct tuple *tuple, union sparse_part* parts)
 {
-	assert (tuple->field_count >= key_def->max_fieldno);
+	assert(tuple->field_count >= key_def->max_fieldno);
 
 	const char *part_data = tuple->data;
 
diff --git a/src/box/tuple.cc b/src/box/tuple.cc
index f2a9591212..8ddcbefa86 100644
--- a/src/box/tuple.cc
+++ b/src/box/tuple.cc
@@ -188,7 +188,7 @@ tuple_print(struct tbuf *buf, const struct tuple *tuple)
 		if (likely(++field_no < tuple->field_count))
 			tbuf_printf(buf, ", ");
 	}
-	assert (field_no == tuple->field_count);
+	assert(field_no == tuple->field_count);
 
 	tbuf_printf(buf, "}");
 }
diff --git a/src/box/tuple_update.cc b/src/box/tuple_update.cc
index 2149863b6e..837b5339c4 100644
--- a/src/box/tuple_update.cc
+++ b/src/box/tuple_update.cc
@@ -35,7 +35,7 @@
 #include <exception.h>
 #include <pickle.h>
 
-/** {{{ UPDATE request implementation.
+/** UPDATE request implementation.
  * UPDATE request is represented by a sequence of operations, each
  * working with a single field. There also are operations which
  * add or remove fields. More than one operation on the same field
@@ -579,7 +579,7 @@ do_update_ops(struct tuple_update *update, char *new_data)
 				new_field = (char *) update->alloc(
 					update->alloc_ctx, op->new_field_len);
 			}
-			assert (op->meta != NULL);
+			assert(op->meta != NULL);
 			op->meta->do_op(&op->arg, old_field, new_field);
 			/* Next op uses previous op output as its input. */
 			old_field = new_field;
@@ -600,7 +600,7 @@ do_update_ops(struct tuple_update *update, char *new_data)
 		total_field_count += field_count;
 	}
 
-	assert (update->new_tuple_fcount == total_field_count);
+	assert(update->new_tuple_fcount == total_field_count);
 }
 
 static void
@@ -648,7 +648,7 @@ tuple_update_prepare(region_alloc_func alloc, void *alloc_ctx,
 
 	struct tuple_update *update = (struct tuple_update *)
 			alloc(alloc_ctx, sizeof(*update));
-	assert (update != NULL);
+	assert(update != NULL);
 	memset(update, 0, sizeof(*update));
 	update->alloc = alloc;
 	update->alloc_ctx = alloc_ctx;
diff --git a/src/lib/bitset/expr.c b/src/lib/bitset/expr.c
index 8c004c5607..dee7d6c6c4 100644
--- a/src/lib/bitset/expr.c
+++ b/src/lib/bitset/expr.c
@@ -160,7 +160,7 @@ int
 bitset_expr_add_param(struct bitset_expr *expr, size_t bitset_id,
 		      bool pre_not)
 {
-	assert (expr->size > 0);
+	assert(expr->size > 0);
 	struct bitset_expr_conj *conj = &expr->conjs[expr->size - 1];
 
 	if (bitset_expr_conj_reserve(expr, conj, conj->size + 1) != 0)
diff --git a/src/lib/bitset/index.c b/src/lib/bitset/index.c
index 7250dcb520..541ac65a92 100644
--- a/src/lib/bitset/index.c
+++ b/src/lib/bitset/index.c
@@ -43,7 +43,7 @@ int
 bitset_index_create(struct bitset_index *index,
 		    void *(*realloc)(void *ptr, size_t size))
 {
-	assert (index != NULL);
+	assert(index != NULL);
 	memset(index, 0, sizeof(*index));
 	index->realloc = realloc;
 	if (bitset_index_reserve(index, 1) != 0)
@@ -55,8 +55,8 @@ bitset_index_create(struct bitset_index *index,
 void
 bitset_index_destroy(struct bitset_index *index)
 {
-	assert (index != NULL);
-	assert (index->capacity > 0);
+	assert(index != NULL);
+	assert(index->capacity > 0);
 
 	for (size_t b = 0; b < index->capacity; b++) {
 		if (index->bitsets[b] == NULL)
@@ -138,9 +138,9 @@ int
 bitset_index_insert(struct bitset_index *index, const void *key,
 		    size_t key_size, size_t value)
 {
-	assert (index != NULL);
-	assert (key != NULL);
-	assert (index->capacity > 0);
+	assert(index != NULL);
+	assert(key != NULL);
+	assert(index->capacity > 0);
 
 	/*
 	 * Step 0: allocate enough number of bitsets
@@ -367,8 +367,8 @@ int
 bitset_index_init_iterator(struct bitset_index *index,
 			   struct bitset_iterator *it, struct bitset_expr *expr)
 {
-	assert (index != NULL);
-	assert (it != NULL);
+	assert(index != NULL);
+	assert(it != NULL);
 
 	/* Check that we have all required bitsets */
 	size_t max = 0;
diff --git a/src/lib/bitset/page.h b/src/lib/bitset/page.h
index 4f519b30c8..2f77925e23 100644
--- a/src/lib/bitset/page.h
+++ b/src/lib/bitset/page.h
@@ -151,7 +151,7 @@ bitset_page_and(struct bitset_page *dst, struct bitset_page *src)
 	bitset_word_t *d = (bitset_word_t *) bitset_page_data(dst);
 	bitset_word_t *s = (bitset_word_t *) bitset_page_data(src);
 
-	assert (BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
+	assert(BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
 	int cnt = BITSET_PAGE_DATA_SIZE / sizeof(bitset_word_t);
 	for (int i = 0; i < cnt; i++) {
 		*d++ &= *s++;
@@ -164,7 +164,7 @@ bitset_page_nand(struct bitset_page *dst, struct bitset_page *src)
 	bitset_word_t *d = (bitset_word_t *) bitset_page_data(dst);
 	bitset_word_t *s = (bitset_word_t *) bitset_page_data(src);
 
-	assert (BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
+	assert(BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
 	int cnt = BITSET_PAGE_DATA_SIZE / sizeof(bitset_word_t);
 	for (int i = 0; i < cnt; i++) {
 		*d++ &= ~*s++;
@@ -177,7 +177,7 @@ bitset_page_or(struct bitset_page *dst, struct bitset_page *src)
 	bitset_word_t *d = (bitset_word_t *) bitset_page_data(dst);
 	bitset_word_t *s = (bitset_word_t *) bitset_page_data(src);
 
-	assert (BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
+	assert(BITSET_PAGE_DATA_SIZE % sizeof(bitset_word_t) == 0);
 	int cnt = BITSET_PAGE_DATA_SIZE / sizeof(bitset_word_t);
 	for (int i = 0; i < cnt; i++) {
 		*d++ |= *s++;
diff --git a/src/memcached.cc b/src/memcached.cc
index a4b0567db1..1fa36b8f37 100644
--- a/src/memcached.cc
+++ b/src/memcached.cc
@@ -182,7 +182,7 @@ memcached_meta(struct tuple *tuple)
 {
 	uint32_t len;
 	const char *field = tuple_field(tuple, 1, &len);
-	assert (sizeof(struct meta) <= len);
+	assert(sizeof(struct meta) <= len);
 	return (struct meta *) field;
 }
 
-- 
GitLab