From 366b2de7cb244fcafeb6d3bc57ed444169b2c433 Mon Sep 17 00:00:00 2001
From: Kirill Yukhin <kyukhin@tarantool.org>
Date: Mon, 18 May 2020 19:12:29 +0300
Subject: [PATCH] Allow to set directory for copying DSO before load

Make it possible to set temporary directory where
module will be copied before load.

@TarantoolBot document
Title: Justify module (re-)loading semantics

It is now possible to set directory where temporary
copies of modules to be loaded will be created.
It is done by setting $(TMPDIR) variable. It
will be "/tmp" if variable was not set.

Follow up #4945
---
 src/box/call.c                | 10 ++++----
 src/box/func.c                | 45 ++++++++++++++++++++---------------
 test/box/func_reload.result   | 30 +++++++++++++++++------
 test/box/func_reload.test.lua | 13 +++++++---
 4 files changed, 65 insertions(+), 33 deletions(-)

diff --git a/src/box/call.c b/src/box/call.c
index 7c70d81693..9c291260e3 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -129,10 +129,12 @@ box_module_reload(const char *name)
 		return -1;
 	}
 	struct module *module = NULL;
-	if (module_reload(name, name + strlen(name), &module) == 0 &&
-	    module != NULL)
-		return 0;
-	diag_set(ClientError, ER_NO_SUCH_MODULE, name);
+	if (module_reload(name, name + strlen(name), &module) == 0) {
+		if (module != NULL)
+			return 0;
+		else
+			diag_set(ClientError, ER_NO_SUCH_MODULE, name);
+	}
 	return -1;
 }
 
diff --git a/src/box/func.c b/src/box/func.c
index c44da9971a..8087c953f5 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -263,14 +263,29 @@ module_load(const char *package, const char *package_end)
 	module->package[package_len] = 0;
 	rlist_create(&module->funcs);
 	module->calls = 0;
-	char dir_name[] = "/tmp/tntXXXXXX";
+
+	const char *tmpdir = getenv("TMPDIR");
+	if (tmpdir == NULL)
+		tmpdir = "/tmp";
+	char dir_name[PATH_MAX];
+	int rc = snprintf(dir_name, sizeof(dir_name), "%s/tntXXXXXX", tmpdir);
+	if (rc < 0 || (size_t) rc >= sizeof(dir_name)) {
+		diag_set(SystemError, "failed to generate path to tmp dir");
+		goto error;
+	}
+
 	if (mkdtemp(dir_name) == NULL) {
-		diag_set(SystemError, "failed to create unique dir name");
+		diag_set(SystemError, "failed to create unique dir name: %s",
+			 dir_name);
+		goto error;
+	}
+	char load_name[PATH_MAX];
+	rc = snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
+		      dir_name, package_len, package);
+	if (rc < 0 || (size_t) rc >= sizeof(dir_name)) {
+		diag_set(SystemError, "failed to generate path to DSO");
 		goto error;
 	}
-	char load_name[PATH_MAX + 1];
-	snprintf(load_name, sizeof(load_name), "%s/%.*s." TARANTOOL_LIBEXT,
-		 dir_name, package_len, package);
 
 	struct stat st;
 	if (stat(path, &st) < 0) {
@@ -293,22 +308,14 @@ module_load(const char *package, const char *package_end)
 		goto error;
 	}
 
-	off_t pos, left;
-	for (left = st.st_size, pos = 0; left > 0;) {
-		off_t ret = eio_sendfile_sync(dest_fd, source_fd, pos,
-					      st.st_size);
-		if (ret < 0) {
-			diag_set(SystemError, "failed to copy DSO %s to %s",
-				 path, load_name);
-			close(source_fd);
-			close(dest_fd);
-			goto error;
-		}
-		pos += ret;
-		left -= ret;
-	}
+	off_t ret = eio_sendfile_sync(dest_fd, source_fd, 0, st.st_size);
 	close(source_fd);
 	close(dest_fd);
+	if (ret != st.st_size) {
+		diag_set(SystemError, "failed to copy DSO %s to %s",
+			 path, load_name);
+		goto error;
+	}
 
 	module->handle = dlopen(load_name, RTLD_NOW | RTLD_LOCAL);
 	if (unlink(load_name) != 0)
diff --git a/test/box/func_reload.result b/test/box/func_reload.result
index 1313fdf184..fbc2ad75df 100644
--- a/test/box/func_reload.result
+++ b/test/box/func_reload.result
@@ -233,20 +233,36 @@ c:call("reload.test_reload_fail")
 ---
 - [[2]]
 ...
-box.schema.func.drop("reload.test_reload")
+box.schema.func.reload()
 ---
+- error: 'bad argument #1 to ''?'' (string expected, got no value)'
 ...
-box.schema.func.drop("reload.test_reload_fail")
+box.schema.func.reload("non-existing")
 ---
+- error: Module 'non-existing' does not exist
 ...
-_ = fio.unlink(reload_path)
+-- Make sure that $TMPDIR env variable is used to generate temporary
+-- path for DSO copy
+os.setenv("TMPDIR", "/dev/null")
 ---
 ...
-box.schema.func.reload()
+_, err = pcall(box.schema.func.reload, "reload")
 ---
-- error: 'bad argument #1 to ''?'' (string expected, got no value)'
 ...
-box.schema.func.reload("non-existing")
+tostring(err):gsub(': [/%w]+$', '')
+---
+- failed to create unique dir name
+- 1
+...
+os.setenv("TMPDIR", nil)
+---
+...
+box.schema.func.drop("reload.test_reload")
+---
+...
+box.schema.func.drop("reload.test_reload_fail")
+---
+...
+_ = fio.unlink(reload_path)
 ---
-- error: Module 'non-existing' does not exist
 ...
diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua
index 4c062fd72a..5b1b4e901c 100644
--- a/test/box/func_reload.test.lua
+++ b/test/box/func_reload.test.lua
@@ -79,9 +79,16 @@ fio.copyfile(reload1_path, reload_path)
 c:call("reload.test_reload")
 c:call("reload.test_reload_fail")
 
+box.schema.func.reload()
+box.schema.func.reload("non-existing")
+
+-- Make sure that $TMPDIR env variable is used to generate temporary
+-- path for DSO copy
+os.setenv("TMPDIR", "/dev/null")
+_, err = pcall(box.schema.func.reload, "reload")
+tostring(err):gsub(': [/%w]+$', '')
+os.setenv("TMPDIR", nil)
+
 box.schema.func.drop("reload.test_reload")
 box.schema.func.drop("reload.test_reload_fail")
 _ = fio.unlink(reload_path)
-
-box.schema.func.reload()
-box.schema.func.reload("non-existing")
-- 
GitLab