From 01af6a683d8d6aae2efe3b1ba32f44da9f44f00a Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Fri, 27 Nov 2015 00:59:50 +0300
Subject: [PATCH] arm: lua/call.cc split, review fixes

---
 src/box/box.cc      | 82 ++++++++++++++++++++++++++++++++++++++-------
 src/box/box.h       |  3 ++
 src/box/func.cc     | 57 -------------------------------
 src/box/func.h      |  7 ----
 src/box/iproto.cc   |  4 +--
 src/box/key_def.h   |  7 ++++
 src/box/lua/call.cc | 23 ++-----------
 src/box/lua/call.h  |  2 +-
 src/box/request.h   |  7 ----
 9 files changed, 85 insertions(+), 107 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index f0b8548b5a..63a3bad5b8 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -40,7 +40,6 @@
 #include "main.h"
 #include "tuple.h"
 #include "session.h"
-#include "user.h"
 #include "func.h"
 #include "schema.h"
 #include "engine.h"
@@ -59,6 +58,8 @@
 #include "cluster.h" /* replica */
 #include "title.h"
 #include "lua/call.h" /* box_lua_call */
+#include "iproto_port.h"
+
 
 static char status[64] = "unknown";
 
@@ -673,7 +674,6 @@ access_check_func(const char *name, uint32_t name_len)
 {
 	struct func *func = func_by_name(name, name_len);
 	struct credentials *credentials = current_user();
-
 	/*
 	 * If the user has universal access, don't bother with checks.
 	 * No special check for ADMIN user is necessary
@@ -695,6 +695,56 @@ access_check_func(const char *name, uint32_t name_len)
 	return func;
 }
 
+int
+func_call(struct func *func, struct request *request, struct obuf *out)
+{
+	assert(func != NULL && func->def.language == FUNC_LANGUAGE_C);
+	if (func->func == NULL)
+		func_load(func);
+
+	/* Create a call context */
+	struct port_buf port_buf;
+	port_buf_create(&port_buf);
+	box_function_ctx_t ctx = { request, &port_buf.base };
+
+	/* Clear all previous errors */
+	diag_clear(&fiber()->diag);
+	assert(!in_txn()); /* transaction is not started */
+	/* Call function from the shared library */
+	int rc = func->func(&ctx, request->tuple, request->tuple_end);
+	if (rc != 0) {
+		if (diag_last_error(&fiber()->diag) == NULL) {
+			/* Stored procedure forget to set diag  */
+			diag_set(ClientError, ER_PROC_C, "unknown error");
+		}
+		goto error;
+	}
+
+	/* Push results to obuf */
+	struct obuf_svp svp;
+	if (iproto_prepare_select(out, &svp) != 0)
+		goto error;
+
+	for (struct port_buf_entry *entry = port_buf.first;
+	     entry != NULL; entry = entry->next) {
+		if (tuple_to_obuf(entry->tuple, out) != 0) {
+			obuf_rollback_to_svp(out, &svp);
+			goto error;
+		}
+	}
+	iproto_reply_select(out, &svp, request->header->sync,
+			    port_buf.size);
+
+	port_buf_destroy(&port_buf);
+
+	return 0;
+
+error:
+	port_buf_destroy(&port_buf);
+	txn_rollback();
+	return -1;
+}
+
 void
 box_process_call(struct request *request, struct obuf *out)
 {
@@ -710,12 +760,14 @@ box_process_call(struct request *request, struct obuf *out)
 	 */
 
 	/**
-	 * Change the current user id if the function is a set-definer-uid one.
+	 * Change the current user id if the function is
+	 * a set-definer-uid one. If the function is not
+	 * defined, it's obviously not a setuid one.
 	 */
 	struct credentials *orig_credentials = NULL;
 	if (func && func->def.setuid) {
 		orig_credentials = current_user();
-		/** Remember and change the current user id. */
+		/* Remember and change the current user id. */
 		if (func->owner_credentials.auth_token >= BOX_USER_MAX) {
 			/*
 			 * Fill the cache upon first access, since
@@ -730,16 +782,13 @@ box_process_call(struct request *request, struct obuf *out)
 	}
 
 	int rc;
-	if (func != NULL && func->def.language == FUNC_LANGUAGE_C) {
+	if (func && func->def.language == FUNC_LANGUAGE_C) {
 		rc = func_call(func, request, out);
 	} else {
-		rc = box_lua_call(func, request, out);
+		rc = box_lua_call(request, out);
 	}
-
-	/*
-	 * Restore original user
-	 */
-	if (orig_credentials != NULL)
+	/* Restore the original user */
+	if (orig_credentials)
 		fiber_set_user(fiber(), orig_credentials);
 
 	if (rc != 0) {
@@ -749,12 +798,21 @@ box_process_call(struct request *request, struct obuf *out)
 
 	if (in_txn()) {
 		/* The procedure forgot to call box.commit() */
-		say_warn("a transaction is active at CALL return from '%.*s'",
+		say_warn("a transaction is active at return from '%.*s'",
 			name_len, name);
 		txn_rollback();
 	}
 }
 
+void
+box_process_eval(struct request *request, struct obuf *out)
+{
+	/* Check permissions */
+	access_check_universe(PRIV_X);
+	box_lua_eval(request, out);
+}
+
+
 void
 box_process_join(int fd, struct xrow_header *header)
 {
diff --git a/src/box/box.h b/src/box/box.h
index 1b7571eaa9..5d89bb70c6 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -87,6 +87,9 @@ box_process_auth(struct request *request);
 void
 box_process_call(struct request *request, struct obuf *out);
 
+void
+box_process_eval(struct request *request, struct obuf *out);
+
 void
 box_process_join(int fd, struct xrow_header *header);
 
diff --git a/src/box/func.cc b/src/box/func.cc
index db62c1fd74..a78f9e3b74 100644
--- a/src/box/func.cc
+++ b/src/box/func.cc
@@ -33,14 +33,8 @@
 #include <dlfcn.h>
 
 #include "lua/utils.h"
-#include "diag.h"
 #include "scoped_guard.h"
 
-#include "box/request.h"
-#include "box/txn.h"
-#include "box/port.h"
-#include "box/iproto_port.h"
-
 struct func *
 func_new(struct func_def *def)
 {
@@ -148,54 +142,3 @@ func_delete(struct func *func)
 	func_unload(func);
 	free(func);
 }
-
-int
-func_call(struct func *func, struct request *request, struct obuf *out)
-{
-	assert(func != NULL && func->def.language == FUNC_LANGUAGE_C);
-	if (func->func == NULL)
-		func_load(func);
-
-	/* Create a call context */
-	struct port_buf port_buf;
-	port_buf_create(&port_buf);
-	box_function_ctx_t ctx = { request, &port_buf.base };
-
-	/* Clear all previous errors */
-	diag_clear(&fiber()->diag);
-	assert(!in_txn()); /* transaction is not started */
-	/* Call function from the shared library */
-	int rc = func->func(&ctx, request->tuple, request->tuple_end);
-	if (rc != 0) {
-		if (diag_last_error(&fiber()->diag) == NULL) {
-			/* Stored procedure forget to set diag  */
-			diag_set(ClientError, ER_PROC_C,
-				 "unknown procedure error");
-		}
-		goto error;
-	}
-
-	/* Push results to obuf */
-	struct obuf_svp svp;
-	if (iproto_prepare_select(out, &svp) != 0)
-		goto error;
-
-	for (struct port_buf_entry *entry = port_buf.first;
-	     entry != NULL; entry = entry->next) {
-		if (tuple_to_obuf(entry->tuple, out) != 0) {
-			obuf_rollback_to_svp(out, &svp);
-			goto error;
-		}
-	}
-	iproto_reply_select(out, &svp, request->header->sync,
-			    port_buf.size);
-
-	port_buf_destroy(&port_buf);
-
-	return 0;
-
-error:
-	port_buf_destroy(&port_buf);
-	txn_rollback();
-	return -1;
-}
diff --git a/src/box/func.h b/src/box/func.h
index f7c0f9e12c..72d2870f99 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -31,7 +31,6 @@
  * SUCH DAMAGE.
  */
 #include "key_def.h"
-#include "request.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -62,12 +61,6 @@ struct func {
 	struct access access[BOX_USER_MAX];
 };
 
-struct request;
-struct obuf;
-
-int
-func_call(struct func *func, struct request *request, struct obuf *out);
-
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 6766abbad9..1b09f4868f 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -49,10 +49,8 @@
 #include "xrow.h"
 #include "recovery.h" /* server_uuid */
 #include "iproto_constants.h"
-#include "user_def.h"
 #include "authentication.h"
 #include "rmean.h"
-#include "lua/call.h"
 
 /* {{{ iproto_msg - declaration */
 
@@ -705,7 +703,7 @@ tx_process_msg(struct cmsg *m)
 		case IPROTO_EVAL:
 			assert(msg->request.type == msg->header.type);
 			rmean_collect(rmean_box, msg->request.type, 1);
-			box_lua_eval(&msg->request, out);
+			box_process_eval(&msg->request, out);
 			break;
 		case IPROTO_AUTH:
 		{
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 924ba8319f..888468b480 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -284,6 +284,13 @@ struct space_def {
 	bool temporary;
 };
 
+/**
+ * API of C stored function.
+ */
+typedef struct box_function_ctx box_function_ctx_t;
+typedef int (*box_function_f)(box_function_ctx_t *ctx,
+	     const char *args, const char *args_end);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/lua/call.cc b/src/box/lua/call.cc
index f4d890ea72..0d8899493d 100644
--- a/src/box/lua/call.cc
+++ b/src/box/lua/call.cc
@@ -32,21 +32,8 @@
 
 #include "lua/utils.h"
 #include "lua/msgpack.h"
-#include "iobuf.h"
-#include "fiber.h"
-#include "scoped_guard.h"
 
-#include "box/box.h"
-#include "box/port.h"
-#include "box/request.h"
-#include "box/engine.h"
 #include "box/txn.h"
-#include "box/user_def.h"
-#include "box/user.h"
-#include "box/func.h"
-#include "box/schema.h"
-#include "box/session.h"
-#include "box/iproto_constants.h"
 #include "box/iproto_port.h"
 #include "box/lua/tuple.h"
 
@@ -237,14 +224,12 @@ execute_lua_call(lua_State *L, struct request *request, struct obuf *out)
 }
 
 int
-box_lua_call(struct func *func, struct request *request, struct obuf *out)
+box_lua_call(struct request *request, struct obuf *out)
 {
 	/*
 	 * func == NULL means that perhaps the user has a global
 	 * "EXECUTE" privilege, so no specific grant to a function.
 	 */
-	assert(func == NULL || func->def.language == FUNC_LANGUAGE_LUA);
-	(void) func;
 	lua_State *L = NULL;
 	try {
 		L = lua_newthread(tarantool_L);
@@ -261,11 +246,9 @@ box_lua_call(struct func *func, struct request *request, struct obuf *out)
 	}
 }
 
-static inline void
+void
 execute_eval(lua_State *L, struct request *request, struct obuf *out)
 {
-	/* Check permissions */
-	access_check_universe(PRIV_X);
 
 	/* Compile expression */
 	const char *expr = request->key;
@@ -304,6 +287,7 @@ execute_eval(lua_State *L, struct request *request, struct obuf *out)
 	}
 }
 
+
 void
 box_lua_eval(struct request *request, struct obuf *out)
 {
@@ -321,7 +305,6 @@ box_lua_eval(struct request *request, struct obuf *out)
 	}
 }
 
-
 static const struct luaL_reg boxlib_internal[] = {
 	{"call_loadproc",  lbox_call_loadproc},
 	{NULL, NULL}
diff --git a/src/box/lua/call.h b/src/box/lua/call.h
index cc264c774f..db58f2021e 100644
--- a/src/box/lua/call.h
+++ b/src/box/lua/call.h
@@ -54,7 +54,7 @@ struct obuf;
  * (implementation of 'CALL' command code).
  */
 int
-box_lua_call(struct func *func, struct request *request, struct obuf *out);
+box_lua_call(struct request *request, struct obuf *out);
 
 void
 box_lua_eval(struct request *request, struct obuf *out);
diff --git a/src/box/request.h b/src/box/request.h
index aeb4ed90cc..45d05f88a9 100644
--- a/src/box/request.h
+++ b/src/box/request.h
@@ -98,11 +98,4 @@ void
 request_rebind_to_primary_key(struct request *request, struct space *space,
 			      struct tuple *found_tuple);
 
-/**
- * API of C stored function.
- */
-typedef struct box_function_ctx box_function_ctx_t;
-typedef int (*box_function_f)(box_function_ctx_t *ctx,
-	     const char *args, const char *args_end);
-
 #endif /* TARANTOOL_BOX_REQUEST_H_INCLUDED */
-- 
GitLab