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

[Kirill Shcherbatov]

>>> AFAIU a user may reload a whole module only if he has the global EXECUTE
>>> privilege (because we don't have such an entity as module in our data
>>> dictionary to grant access rights for). access_check_func(), which is
>>> called by func_reload(), already checks the global EXECUTE privilege and
>>> returns 0 (success) if it is set, no matter if the function was found or
>>> not. So all you have to do is call module_reload() from func_reload() if
>>> access_check_func() returned func = NULL, no?
>>
>> No, this call iterates through all loaded functions of specified module and 
>> reload all that belongs to initiator if any.
> 
> I understand what you code does, but I wonder why it does that.
As we have verbally discussed,

==================================

diff --git a/src/box/call.c b/src/box/call.c
index 438be19..b322c03 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -139,14 +139,24 @@ box_func_reload(const char *name)
 	if ((access_check_func(name, name_len, &func)) != 0)
 		return -1;
 	if (func == NULL) {
+		/* Try to interpret name as a module name. */
+		struct module *module;
+		if ((effective_user()->universal_access & (PRIV_X | PRIV_U)) ==
+		    (PRIV_X | PRIV_U) &&
+		    module_reload(name, name + name_len, &module) == 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
diff --git a/src/box/call.h b/src/box/call.h
index eabba69..14844fc 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;
diff --git a/src/box/func.c b/src/box/func.c
index dfbc5f3..ccbc12b 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>
 
 /**
diff --git a/src/box/func.h b/src/box/func.h
index 0957546..3b67b06 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);
+
 #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()