* [Tarantool-patches] [PATCH 1/2] fio: respect $TMPDIR in fio.tempdir(), when it is set
2020-03-01 18:26 [Tarantool-patches] [PATCH 0/2] fio.tempdir() improvements Vladislav Shpilevoy
@ 2020-03-01 18:26 ` Vladislav Shpilevoy
2020-03-01 18:26 ` [Tarantool-patches] [PATCH 2/2] fio: allow to pass a template to fio.tempdir() Vladislav Shpilevoy
2020-03-01 23:14 ` [Tarantool-patches] [PATCH 0/2] fio.tempdir() improvements Igor Munkin
2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-01 18:26 UTC (permalink / raw)
To: tarantool-patches, korablev, imun
TMPDIR is an environment variable used to tell what a directory
should be used to create temporary files. It is described in the
POSIX standard, and should be used by programs creating temporary
files.
Part of #4794
---
src/lib/core/coio_file.c | 11 ++++---
test/app/fio.result | 68 ++++++++++++++++++++++++++++++++++++++++
test/app/fio.test.lua | 28 +++++++++++++++++
3 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index 62388344e..e2345567c 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -466,13 +466,16 @@ coio_tempdir(char *path, size_t path_len)
{
INIT_COEIO_FILE(eio);
- if (path_len < sizeof("/tmp/XXXXXX") + 1) {
+ const char *tmpdir = getenv("TMPDIR");
+ if (tmpdir == NULL)
+ tmpdir = "/tmp";
+ int rc = snprintf(path, path_len, "%s/XXXXXX", tmpdir);
+ if (rc < 0)
+ return -1;
+ if ((size_t) rc >= path_len) {
errno = ENOMEM;
return -1;
}
-
- snprintf(path, path_len, "/tmp/XXXXXX");
-
eio.tempdir.tpl = path;
eio_req *req =
eio_custom(coio_do_tempdir, 0, coio_complete, &eio);
diff --git a/test/app/fio.result b/test/app/fio.result
index f83c43f44..2c9851d9e 100644
--- a/test/app/fio.result
+++ b/test/app/fio.result
@@ -1456,3 +1456,71 @@ fio.mktree('/dev/null/dir')
- false
- 'Error creating directory /dev/null: File exists'
...
+--
+-- gh-4794: fio.tempdir() should use $TEMPDIR.
+--
+cwd = fio.cwd()
+---
+...
+old_tmpdir = os.getenv('TMPDIR')
+---
+...
+tmpdir = cwd..'/tmp'
+---
+...
+fio.mkdir(tmpdir)
+---
+- true
+...
+os.setenv('TMPDIR', tmpdir)
+---
+...
+dir = fio.tempdir()
+---
+...
+dir:find(tmpdir) ~= nil or {dir, tmpdir}
+---
+- true
+...
+fio.stat(dir) ~= nil or fio.stat(dir)
+---
+- true
+...
+tmpdir = cwd..'/tmp2'
+---
+...
+os.setenv('TMPDIR', tmpdir)
+---
+...
+fio.tempdir()
+---
+- null
+- 'fio: No such file or directory'
+...
+os.setenv('TMPDIR', nil)
+---
+...
+dir = fio.tempdir()
+---
+...
+dir:find('/tmp') ~= nil or dir
+---
+- true
+...
+tmpdir = cwd..'/'..string.rep('t', 5000)
+---
+...
+os.setenv('TMPDIR', tmpdir)
+---
+...
+fio.tempdir()
+---
+- null
+- 'fio: Cannot allocate memory'
+...
+tmpdir = nil
+---
+...
+os.setenv('TMPDIR', old_tmpdir)
+---
+...
diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua
index 56c957d8a..fb6a412b3 100644
--- a/test/app/fio.test.lua
+++ b/test/app/fio.test.lua
@@ -474,3 +474,31 @@ test_run:cmd("clear filter")
--
fio.mktree('/dev/null')
fio.mktree('/dev/null/dir')
+
+--
+-- gh-4794: fio.tempdir() should use $TEMPDIR.
+--
+cwd = fio.cwd()
+old_tmpdir = os.getenv('TMPDIR')
+
+tmpdir = cwd..'/tmp'
+fio.mkdir(tmpdir)
+os.setenv('TMPDIR', tmpdir)
+dir = fio.tempdir()
+dir:find(tmpdir) ~= nil or {dir, tmpdir}
+fio.stat(dir) ~= nil or fio.stat(dir)
+
+tmpdir = cwd..'/tmp2'
+os.setenv('TMPDIR', tmpdir)
+fio.tempdir()
+
+os.setenv('TMPDIR', nil)
+dir = fio.tempdir()
+dir:find('/tmp') ~= nil or dir
+
+tmpdir = cwd..'/'..string.rep('t', 5000)
+os.setenv('TMPDIR', tmpdir)
+fio.tempdir()
+tmpdir = nil
+
+os.setenv('TMPDIR', old_tmpdir)
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Tarantool-patches] [PATCH 2/2] fio: allow to pass a template to fio.tempdir()
2020-03-01 18:26 [Tarantool-patches] [PATCH 0/2] fio.tempdir() improvements Vladislav Shpilevoy
2020-03-01 18:26 ` [Tarantool-patches] [PATCH 1/2] fio: respect $TMPDIR in fio.tempdir(), when it is set Vladislav Shpilevoy
@ 2020-03-01 18:26 ` Vladislav Shpilevoy
2020-03-01 23:14 ` [Tarantool-patches] [PATCH 0/2] fio.tempdir() improvements Igor Munkin
2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-01 18:26 UTC (permalink / raw)
To: tarantool-patches, korablev, imun
Rules are the same as for mkdtemp() POSIX function. Template
should have at least 6 'X' symbols in the end. Last 6 are
replaced with random printable characters: digits or letters.
Closes #4794
@TarantoolBot document
Title: fio.tempdir() $TMPDIR and custom template
fio.tempdir() stores created temporary directory into /tmp by
default. This can be changed by setting TMPDIR environment
variable. Before starting Tarantool, or at runtime by
os.setenv().
fio.tempdir() now accepts a template parameter. It should be a
dirname having 'XXXXXX' at the end. These 'X' are replaced with
random digits and letters. Dirname should not include any other
path parts. Path to the dir to create is set by TMPDIR env var.
Template should have at least 6 'X' in the end. If it has less,
it is an error. If it has more, only 6 last 'X' are replaced. So,
for example, this template 'dirXXXXXXXX' can turn into
'dirXXabcdef'. First 'XX' is not touched.
Default template is 'XXXXXX'.
---
src/lib/core/coio_file.c | 67 +++++++++++++++++++++++++++++++++++-----
src/lib/core/coio_file.h | 2 +-
src/lua/fio.c | 3 +-
test/app/fio.result | 55 ++++++++++++++++++++++++++++++++-
test/app/fio.test.lua | 18 ++++++++++-
5 files changed, 134 insertions(+), 11 deletions(-)
diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index e2345567c..adb2fe4b0 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -34,6 +34,7 @@
#include "say.h"
#include "fio.h"
#include "errinj.h"
+#include "random.h"
#include <stdio.h>
#include <stdlib.h>
#include <dirent.h>
@@ -448,28 +449,80 @@ coio_readlink(const char *pathname, char *buf, size_t bufsize)
return coio_wait_done(req, &eio);
}
+/**
+ * glibc implementation of mkdtemp(). mkdtemp() is not used
+ * directly, because its behaviour differs on systems. For
+ * example, on Mac mkdtemp() behaves in a non standard way - it
+ * allows a template to have more and less than 6 'X' symbols.
+ * When more than 6, all of them are replaced. On Linux it is only
+ * 6 'X'. Less is treated as an error. More than 6 does not affect
+ * behaviour - only last 6 'X' are replaced.
+ */
static void
coio_do_tempdir(eio_req *req)
{
struct coio_file_task *eio = (struct coio_file_task *)req->data;
- char *res = mkdtemp(eio->tempdir.tpl);
- req->errorno = errno;
- if (res == NULL) {
+ char *templ = eio->tempdir.tpl;
+ /*
+ * A lower bound on the number of temporary files to
+ * attempt to generate. The maximum total number of
+ * temporary file names that can exist for a given
+ * template is 62**6. It should never be necessary to try
+ * all of these combinations. Instead if a reasonable
+ * number of names is tried (62**3) fail to give the
+ * system administrator the chance to remove the problems.
+ */
+ int attempts = 62 * 62 * 62;
+ if (attempts < TMP_MAX)
+ attempts = TMP_MAX;
+ int len = strlen(templ);
+ if (len < 6 || memcmp(&templ[len - 6], "XXXXXX", 6) != 0) {
+ errno = EINVAL;
req->result = -1;
- } else {
- req->result = 0;
+ return;
}
+ char *xxxxxx = &templ[len - 6];
+ const char *letters = "abcdefghijklmnopqrstuvwxyz"
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+ uint64_t value;
+ random_bytes((char *)&value, sizeof(value));
+ for (int count = 0; count < attempts; value += 7777, ++count) {
+ uint64_t v = value;
+ xxxxxx[0] = letters[v % 62];
+ v /= 62;
+ xxxxxx[1] = letters[v % 62];
+ v /= 62;
+ xxxxxx[2] = letters[v % 62];
+ v /= 62;
+ xxxxxx[3] = letters[v % 62];
+ v /= 62;
+ xxxxxx[4] = letters[v % 62];
+ v /= 62;
+ xxxxxx[5] = letters[v % 62];
+ if (mkdir(templ, S_IRUSR | S_IWUSR | S_IXUSR) == 0) {
+ req->result = 0;
+ return;
+ }
+ if (errno != EEXIST) {
+ req->result = -1;
+ return;
+ }
+ }
+ errno = EEXIST;
+ req->result = -1;
}
int
-coio_tempdir(char *path, size_t path_len)
+coio_tempdir(char *path, size_t path_len, const char *templ)
{
INIT_COEIO_FILE(eio);
const char *tmpdir = getenv("TMPDIR");
if (tmpdir == NULL)
tmpdir = "/tmp";
- int rc = snprintf(path, path_len, "%s/XXXXXX", tmpdir);
+ if (templ == NULL)
+ templ = "XXXXXX";
+ int rc = snprintf(path, path_len, "%s/%s", tmpdir, templ);
if (rc < 0)
return -1;
if ((size_t) rc >= path_len) {
diff --git a/src/lib/core/coio_file.h b/src/lib/core/coio_file.h
index ac7b1aacf..416cc499c 100644
--- a/src/lib/core/coio_file.h
+++ b/src/lib/core/coio_file.h
@@ -80,7 +80,7 @@ int coio_sync();
int coio_fsync(int fd);
int coio_fdatasync(int fd);
-int coio_tempdir(char *path, size_t path_len);
+int coio_tempdir(char *path, size_t path_len, const char *templ);
int coio_readdir(const char *path, char **buf);
int coio_copyfile(const char *source, const char *dest);
diff --git a/src/lua/fio.c b/src/lua/fio.c
index 7437cc0c6..a3b63bbfb 100644
--- a/src/lua/fio.c
+++ b/src/lua/fio.c
@@ -603,13 +603,14 @@ usage:
static int
lbox_fio_tempdir(struct lua_State *L)
{
+ const char *templ = luaL_optstring(L, 1, NULL);
char *buf = (char *)lua_newuserdata(L, PATH_MAX);
if (!buf) {
errno = ENOMEM;
return lbox_fio_pushsyserror(L);
}
- if (coio_tempdir(buf, PATH_MAX) != 0)
+ if (coio_tempdir(buf, PATH_MAX, templ) != 0)
return lbox_fio_pushsyserror(L);
lua_pushstring(L, buf);
lua_remove(L, -2);
diff --git a/test/app/fio.result b/test/app/fio.result
index 2c9851d9e..2d37ca368 100644
--- a/test/app/fio.result
+++ b/test/app/fio.result
@@ -1457,7 +1457,8 @@ fio.mktree('/dev/null/dir')
- 'Error creating directory /dev/null: File exists'
...
--
--- gh-4794: fio.tempdir() should use $TEMPDIR.
+-- gh-4794: fio.tempdir() should use $TEMPDIR, and accept a
+-- template.
--
cwd = fio.cwd()
---
@@ -1524,3 +1525,55 @@ tmpdir = nil
os.setenv('TMPDIR', old_tmpdir)
---
...
+dir, err = fio.tempdir('dirXXXXXX')
+---
+...
+err
+---
+- null
+...
+dir:find('XXXXXX') == nil or dir
+---
+- true
+...
+dir, err = fio.tempdir('dirXXXXXXXX')
+---
+...
+err
+---
+- null
+...
+dir:find('XXXXXX') == nil and dir:find('XX') ~= nil or dir
+---
+- true
+...
+fio.tempdir('')
+---
+- null
+- 'fio: Invalid argument'
+...
+fio.tempdir('XXX')
+---
+- null
+- 'fio: Invalid argument'
+...
+fio.tempdir('XXXXXXdir')
+---
+- null
+- 'fio: Invalid argument'
+...
+fio.tempdir(100)
+---
+- null
+- 'fio: Invalid argument'
+...
+fio.tempdir('path/dirXXXXXX')
+---
+- null
+- 'fio: No such file or directory'
+...
+fio.tempdir('/tmp/dirXXXXXX')
+---
+- null
+- 'fio: No such file or directory'
+...
diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua
index fb6a412b3..df8f3a122 100644
--- a/test/app/fio.test.lua
+++ b/test/app/fio.test.lua
@@ -476,7 +476,8 @@ fio.mktree('/dev/null')
fio.mktree('/dev/null/dir')
--
--- gh-4794: fio.tempdir() should use $TEMPDIR.
+-- gh-4794: fio.tempdir() should use $TEMPDIR, and accept a
+-- template.
--
cwd = fio.cwd()
old_tmpdir = os.getenv('TMPDIR')
@@ -502,3 +503,18 @@ fio.tempdir()
tmpdir = nil
os.setenv('TMPDIR', old_tmpdir)
+
+dir, err = fio.tempdir('dirXXXXXX')
+err
+dir:find('XXXXXX') == nil or dir
+
+dir, err = fio.tempdir('dirXXXXXXXX')
+err
+dir:find('XXXXXX') == nil and dir:find('XX') ~= nil or dir
+
+fio.tempdir('')
+fio.tempdir('XXX')
+fio.tempdir('XXXXXXdir')
+fio.tempdir(100)
+fio.tempdir('path/dirXXXXXX')
+fio.tempdir('/tmp/dirXXXXXX')
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 4+ messages in thread