From 2dd025fec529cc5162067834573545a6773a225c Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Fri, 7 Nov 2014 17:11:49 +0300
Subject: [PATCH] Minor code refactoring.

Encapsulate error handling in user cache search functions.
---
 src/box/alter.cc          |  7 ++-----
 src/box/authentication.cc |  8 +-------
 src/box/lua/session.cc    | 11 ++---------
 src/box/schema.h          |  5 +----
 src/box/user_cache.cc     | 19 +++++++++++++++++--
 src/box/user_cache.h      |  7 +++++--
 src/trivia/util.h         |  2 +-
 src/util.cc               |  4 ++--
 test/box/alter.result     |  2 +-
 test/box/errinj.result    |  2 +-
 test/box/sql.result       |  6 +++---
 11 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 2a945247cd..284de989ab 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1436,12 +1436,9 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
 static void
 priv_def_check(struct priv_def *priv)
 {
-	struct user_def *grantor = user_by_id(priv->grantor_id);
+	struct user_def *grantor = user_cache_find(priv->grantor_id);
+	/* May be a role */
 	struct user_def *grantee = user_by_id(priv->grantee_id);
-	if (grantor == NULL) {
-		tnt_raise(ClientError, ER_NO_SUCH_USER,
-			  int2str(priv->grantor_id));
-	}
 	if (grantee == NULL) {
 		tnt_raise(ClientError, ER_NO_SUCH_USER,
 			  int2str(priv->grantee_id));
diff --git a/src/box/authentication.cc b/src/box/authentication.cc
index 7f83a3d606..477063df33 100644
--- a/src/box/authentication.cc
+++ b/src/box/authentication.cc
@@ -34,13 +34,7 @@ void
 authenticate(const char *user_name, uint32_t len,
 	     const char *tuple, const char * /* tuple_end */)
 {
-	struct user_def *user = user_by_name(user_name, len);
-	if (user == NULL) {
-		char name[BOX_NAME_MAX + 1];
-		/* \0 - to correctly print user name the error message. */
-		snprintf(name, sizeof(name), "%.*s", len, user_name);
-		tnt_raise(ClientError, ER_NO_SUCH_USER, name);
-	}
+	struct user_def *user = user_cache_find_by_name(user_name, len);
 	struct session *session = session();
 	uint32_t part_count = mp_decode_array(&tuple);
 	if (part_count < 2) {
diff --git a/src/box/lua/session.cc b/src/box/lua/session.cc
index f921790a24..f50bd3eaf1 100644
--- a/src/box/lua/session.cc
+++ b/src/box/lua/session.cc
@@ -93,16 +93,9 @@ lbox_session_su(struct lua_State *L)
 	if (lua_type(L, 1) == LUA_TSTRING) {
 		size_t len;
 		const char *name = lua_tolstring(L, 1, &len);
-		user = user_by_name(name, len);
-		if (user == NULL)
-			tnt_raise(ClientError, ER_NO_SUCH_USER, name);
+		user = user_cache_find_by_name(name, len);
 	} else {
-		uint32_t uid = lua_tointeger(L, 1);;
-		user = user_by_id(uid);
-		if (user == NULL) {
-			tnt_raise(ClientError, ER_NO_SUCH_USER,
-				  int2str(uid));
-		}
+		user = user_cache_find(lua_tointeger(L, 1));
 	}
 	session_set_user(session, user->auth_token, user->uid);
 	return 0;
diff --git a/src/box/schema.h b/src/box/schema.h
index 69b63f66b9..cbd77926c2 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -81,10 +81,7 @@ space_cache_find(uint32_t id)
 	struct space *space = space_by_id(id);
 	if (space)
 		return space;
-
-	char name[12];
-	snprintf(name, sizeof(name), "#%u", id);
-	tnt_raise(ClientError, ER_NO_SUCH_SPACE, name);
+	tnt_raise(ClientError, ER_NO_SUCH_SPACE, int2str(id));
 }
 
 /**
diff --git a/src/box/user_cache.cc b/src/box/user_cache.cc
index 343c63945b..d82555cd3b 100644
--- a/src/box/user_cache.cc
+++ b/src/box/user_cache.cc
@@ -118,13 +118,28 @@ user_by_id(uint32_t uid)
 	return (struct user_def *) mh_i32ptr_node(user_registry, k)->val;
 }
 
+struct user_def *
+user_cache_find(uint32_t uid)
+{
+	struct user_def *user = user_by_id(uid);
+	if (user)
+		return user;
+	tnt_raise(ClientError, ER_NO_SUCH_USER, int2str(uid));
+}
+
 /** Find user by name. */
 struct user_def *
-user_by_name(const char *name, uint32_t len)
+user_cache_find_by_name(const char *name, uint32_t len)
 {
 	uint32_t uid = schema_find_id(SC_USER_ID, 2, name, len);
 	struct user_def *user = user_by_id(uid);
-	return user && user->type == SC_USER ? user : NULL;
+	if (user == NULL || user->type != SC_USER) {
+		char name_buf[BOX_NAME_MAX + 1];
+		/* \0 - to correctly print user name the error message. */
+		snprintf(name_buf, sizeof(name_buf), "%.*s", len, name);
+		tnt_raise(ClientError, ER_NO_SUCH_USER, name_buf);
+	}
+	return user;
 }
 
 void
diff --git a/src/box/user_cache.h b/src/box/user_cache.h
index 8b2a4d22d6..38d3292edc 100644
--- a/src/box/user_cache.h
+++ b/src/box/user_cache.h
@@ -72,7 +72,10 @@ user_by_id(uint32_t uid);
 
 /* Find a user by name. Used by authentication. */
 struct user_def *
-user_by_name(const char *name, uint32_t len);
+user_cache_find(uint32_t uid);
+
+struct user_def *
+user_cache_find_by_name(const char *name, uint32_t len);
 
 /**
  * Return the current user.
@@ -80,7 +83,7 @@ user_by_name(const char *name, uint32_t len);
 #define user()							\
 ({								\
 	struct session *s = session();				\
-	struct user_def *u = &users[s->auth_token];			\
+	struct user_def *u = &users[s->auth_token];		\
 	if (u->auth_token != s->auth_token ||			\
 	    u->uid != s->uid) {					\
 		tnt_raise(ClientError, ER_NO_SUCH_USER,		\
diff --git a/src/trivia/util.h b/src/trivia/util.h
index f6d1a7f49e..d14e80249b 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -172,7 +172,7 @@ char *find_path(const char *argv0);
 char *abspath(const char *filename);
 
 char *
-int2str(int val);
+int2str(long int val);
 
 #ifndef HAVE_MEMMEM
 /* Declare memmem(). */
diff --git a/src/util.cc b/src/util.cc
index f0cd3ca37c..557beb94a6 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -367,10 +367,10 @@ abspath(const char *filename)
 }
 
 char *
-int2str(int val)
+int2str(long int val)
 {
 	static char __thread buf[22];
-	snprintf(buf, sizeof(buf), "%d", val);
+	snprintf(buf, sizeof(buf), "%ld", val);
 	return buf;
 }
 
diff --git a/test/box/alter.result b/test/box/alter.result
index e5974dc819..06f8a885e1 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -134,7 +134,7 @@ space_deleted
 ...
 space:replace{0}
 ---
-- error: Space '#321' does not exist
+- error: Space '321' does not exist
 ...
 _index:insert{_space.id, 0, 'primary', 'tree', 1, 1, 0, 'num'}
 ---
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 204d1ddda9..5435850f50 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -271,7 +271,7 @@ box.space['withdata']
 ...
 index7 = s_withdata:create_index('another', { type = 'tree', parts = { 5, 'num' }, unique = false})
 ---
-- error: Space '#514' does not exist
+- error: Space '514' does not exist
 ...
 s_withdata.index.another
 ---
diff --git a/test/box/sql.result b/test/box/sql.result
index 9c1c375829..9bab5ad7f6 100644
--- a/test/box/sql.result
+++ b/test/box/sql.result
@@ -168,19 +168,19 @@ select * from t1 where k0 = 0
 ---
 - error:
     errcode: ER_NO_SUCH_SPACE
-    errmsg: Space '#1' does not exist
+    errmsg: Space '1' does not exist
 ...
 select * from t65537 where k0 = 0
 ---
 - error:
     errcode: ER_NO_SUCH_SPACE
-    errmsg: Space '#65537' does not exist
+    errmsg: Space '65537' does not exist
 ...
 select * from t4294967295 where k0 = 0
 ---
 - error:
     errcode: ER_NO_SUCH_SPACE
-    errmsg: Space '#4294967295' does not exist
+    errmsg: Space '4294967295' does not exist
 ...
 box.space[0]:drop()
 ---
-- 
GitLab