[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Tnx for comment; I've changed commit message and fixed few 
codestyle things which I missed yesterday.

Closes #2946.

@TarantoolBot document
Title: fixed module reload
There was a bug in tarantool documentation:
https://tarantool.io/en/doc/1.7/book/box/
box_schema/#lua-function.box.schema.func.reload
Now it is allowed to reload all functions in loadable
module via one method.
box.schema.func.reload("utils")    -- ok since now

Global reload is still unsupported because it seems
to be useless.
box.schema.func.reload()           -- invalid!
---
https://github.com/tarantool/tarantool/compare/kshch/gh-2946-module-reload
https://github.com/tarantool/tarantool/issues/2946

 src/box/call.c                | 37 ++++++++++++++++---------------------
 src/box/call.h                | 18 ++++++++++++++++++
 src/box/func.c                | 20 +++++++++++++++++---
 src/box/func.h                | 15 +++++++++++++++
 test/box/func_reload.result   |  7 +++++++
 test/box/func_reload.test.lua |  2 ++
 6 files changed, 75 insertions(+), 24 deletions(-)

diff --git a/src/box/call.c b/src/box/call.c
index 438be19..ef30abc 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -42,20 +42,8 @@
 #include "rmean.h"
 #include "small/obuf.h"
 
-/**
- * Find a function by name and check "EXECUTE" permissions.
- *
- * @param name function name
- * @param name_len length of @a name
- * @param[out] funcp function object
- * Sic: *pfunc == NULL means that perhaps the user has a global
- * "EXECUTE" privilege, so no specific grant to a function.
- *
- * @retval -1 on access denied
- * @retval  0 on success
- */
-static inline int
-access_check_func(const char *name, uint32_t name_len, struct func **funcp)
+int
+box_func_check_access(const char *name, uint32_t name_len, struct func **funcp)
 {
 	struct func *func = func_by_name(name, name_len);
 	struct credentials *credentials = effective_user();
@@ -136,17 +124,24 @@ box_func_reload(const char *name)
 {
 	size_t name_len = strlen(name);
 	struct func *func = NULL;
-	if ((access_check_func(name, name_len, &func)) != 0)
+	if ((box_func_check_access(name, name_len, &func)) != 0)
 		return -1;
 	if (func == NULL) {
+		/* Try to interpret name as a module name. */
+		struct module *module;
+		if (module_reload(name, name + name_len, &module, true) == 0 &&
+		    module != NULL)
+			return 0;
 		diag_set(ClientError, ER_NO_SUCH_FUNCTION, name);
 		return -1;
+	} else {
+		/* Such function exists. Try to reload it. */
+		if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
+			return 0;
+		if (func_reload(func) == 0)
+			return 0;
+		return -1;
 	}
-	if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
-		return 0; /* Nothing to do */
-	if (func_reload(func) == 0)
-		return 0;
-	return -1;
 }
 
 int
@@ -165,7 +160,7 @@ box_process_call(struct call_request *request, struct port *port)
 	 * Sic: func == NULL means that perhaps the user has a global
 	 * "EXECUTE" privilege, so no specific grant to a function.
 	 */
-	if (access_check_func(name, name_len, &func) != 0)
+	if (box_func_check_access(name, name_len, &func) != 0)
 		return -1; /* permission denied */
 
 	/**
diff --git a/src/box/call.h b/src/box/call.h
index eabba69..55cf925 100644
--- a/src/box/call.h
+++ b/src/box/call.h
@@ -35,8 +35,11 @@
 extern "C" {
 #endif /* defined(__cplusplus) */
 
+#include <inttypes.h>
+
 struct port;
 struct call_request;
+struct func;
 
 struct box_function_ctx {
 	struct port *port;
@@ -51,6 +54,21 @@ box_process_call(struct call_request *request, struct port *port);
 int
 box_process_eval(struct call_request *request, struct port *port);
 
+/**
+ * Find a function by name and check "EXECUTE" permissions.
+ *
+ * @param name function name
+ * @param name_len length of @a name
+ * @param[out] funcp function object
+ * Sic: *pfunc == NULL means that perhaps the user has a global
+ * "EXECUTE" privilege, so no specific grant to a function.
+ *
+ * @retval -1 on access denied.
+ * @retval  0 on success.
+ */
+int
+box_func_check_access(const char *name, uint32_t name_len, struct func **funcp);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/func.c b/src/box/func.c
index dfbc5f3..245dbe7 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -34,6 +34,7 @@
 #include "lua/utils.h"
 #include "error.h"
 #include "diag.h"
+#include "call.h"
 #include <dlfcn.h>
 
 /**
@@ -302,7 +303,8 @@ module_sym(struct module *module, const char *name)
  * Reload a dso.
  */
 int
-module_reload(const char *package, const char *package_end, struct module **module)
+module_reload(const char *package, const char *package_end,
+	      struct module **module, bool check_access)
 {
 	struct module *old_module = module_cache_find(package, package_end);
 	if (old_module == NULL) {
@@ -318,7 +320,19 @@ module_reload(const char *package, const char *package_end, struct module **modu
 	struct func *func, *tmp_func;
 	rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) {
 		struct func_name name;
-		func_split_name(func->def->name, &name);
+		const char *func_name = func->def->name;
+		func_split_name(func_name, &name);
+
+		/*
+		 * Allow to reload only functions that belongs to
+		 * current user. Skip other.
+		 */
+		struct func *dummy;
+		if (check_access &&
+		    box_func_check_access(func_name, strlen(func_name),
+					  &dummy) != 0)
+			continue;
+
 		func->func = module_sym(new_module, name.sym);
 		if (func->func == NULL)
 			goto restore;
@@ -441,7 +455,7 @@ func_reload(struct func *func)
 	struct func_name name;
 	func_split_name(func->def->name, &name);
 	struct module *module = NULL;
-	if (module_reload(name.package, name.package_end, &module) != 0) {
+	if (module_reload(name.package, name.package_end, &module, false) != 0) {
 		diag_log();
 		return -1;
 	}
diff --git a/src/box/func.h b/src/box/func.h
index 0957546..168c2fc 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -116,6 +116,21 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 int
 func_reload(struct func *func);
 
+/**
+ * Reload dynamically loadable module.
+ *
+ * @param package name begin pointer.
+ * @param package_end name end pointer.
+ * @param[out] module a pointer to store module object on success.
+ * @param check_access do ACL checks if specified.
+ *
+ * @retval -1 on error
+ * @retval 0 on success
+ */
+int
+module_reload(const char *package, const char *package_end,
+	      struct module **module, bool check_access);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/test/box/func_reload.result b/test/box/func_reload.result
index 5aeb85c..9c05555 100644
--- a/test/box/func_reload.result
+++ b/test/box/func_reload.result
@@ -54,6 +54,10 @@ fio.symlink(reload1_path, reload_path)
 box.schema.func.reload("reload.foo")
 ---
 ...
+box.schema.func.reload("reload")
+---
+- error: Function 'reload' does not exist
+...
 -- test of usual case reload. No hanging calls
 box.space.test:insert{0}
 ---
@@ -77,6 +81,9 @@ fio.symlink(reload2_path, reload_path)
 box.schema.func.reload("reload.foo")
 ---
 ...
+box.schema.func.reload("reload")
+---
+...
 c:call("reload.foo")
 ---
 - []
diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua
index dc56d84..5569dcd 100644
--- a/test/box/func_reload.test.lua
+++ b/test/box/func_reload.test.lua
@@ -21,6 +21,7 @@ fio.symlink(reload1_path, reload_path)
 
 --check not fail on non-load func
 box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 
 -- test of usual case reload. No hanging calls
 box.space.test:insert{0}
@@ -30,6 +31,7 @@ _ = fio.unlink(reload_path)
 fio.symlink(reload2_path, reload_path)
 
 box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 c:call("reload.foo")
 box.space.test:select{}
 box.space.test:truncate()
-- 
2.7.4