| Message ID | 20221101000841.1207999-1-edliaw@google.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] tst_tmpdir: add TST_GET_TMPDIR_ROOT() to get the TMPDIR env var | expand |
Hi! > diff --git a/include/old/old_tmpdir.h b/include/old/old_tmpdir.h > index 9c61172fd..b1cf79344 100644 > --- a/include/old/old_tmpdir.h > +++ b/include/old/old_tmpdir.h > @@ -45,6 +45,11 @@ void tst_rmdir(void); > */ > char *tst_get_tmpdir(void); > > +/* > + * Returns path to the test temporary directory root. > + */ > +const char *tst_get_tmpdir_root(void); > + > /* > * Returns 1 if temp directory was created. > */ > diff --git a/include/tst_test.h b/include/tst_test.h > index a69965b95..26a5ec752 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -362,6 +362,11 @@ void tst_set_max_runtime(int max_runtime); > */ > char *tst_get_tmpdir(void); > > +/* > + * Returns path to the test temporary directory root. > + */ > +const char *tst_get_tmpdir_root(void); > + > /* > * Validates exit status of child processes > */ > diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c > index 6642d6be4..50699bc63 100644 > --- a/lib/tst_cgroup.c > +++ b/lib/tst_cgroup.c > @@ -645,10 +645,7 @@ static void cgroup_mount_v2(void) > { > int ret; > char mnt_path[PATH_MAX]; > - const char *tmpdir = getenv("TMPDIR"); > - > - if (!tmpdir) > - tmpdir = "/tmp"; > + const char *tmpdir = tst_get_tmpdir_root(); > > sprintf(mnt_path, "%s/%s%s", > tmpdir, cgroup_mount_ltp_prefix, cgroup_v2_ltp_mount); > @@ -698,10 +695,7 @@ static void cgroup_mount_v1(struct cgroup_ctrl *const ctrl) > { > char mnt_path[PATH_MAX]; > int made_dir = 0; > - const char *tmpdir = getenv("TMPDIR"); > - > - if (!tmpdir) > - tmpdir = "/tmp"; > + const char *tmpdir = tst_get_tmpdir_root(); > > if (ctrl->ctrl_indx == CTRL_BLKIO && controllers[CTRL_IO].ctrl_root) { > tst_res(TCONF, > diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c > index 7781f94c3..d4911fa3b 100644 > --- a/lib/tst_supported_fs_types.c > +++ b/lib/tst_supported_fs_types.c > @@ -74,14 +74,11 @@ int tst_fs_in_skiplist(const char *fs_type, const char *const *skiplist) > static enum tst_fs_impl has_kernel_support(const char *fs_type) > { > static int fuse_supported = -1; > - const char *tmpdir = getenv("TMPDIR"); > + const char *tmpdir = tst_get_tmpdir_root(); > char buf[128]; > char template[PATH_MAX]; > int ret; > > - if (!tmpdir) > - tmpdir = "/tmp"; > - > snprintf(template, sizeof(template), "%s/mountXXXXXX", tmpdir); > if (!mkdtemp(template)) > tst_brk(TBROK | TERRNO, "mkdtemp(%s) failed", template); > diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c > index 6e38ae977..9c90f481a 100644 > --- a/lib/tst_tmpdir.c > +++ b/lib/tst_tmpdir.c > @@ -122,6 +122,28 @@ char *tst_get_tmpdir(void) > return ret; > } > > +const char *tst_get_tmpdir_root(void) > +{ > + char *c; > + const char *env_tmpdir = getenv("TMPDIR"); > + > + if (!env_tmpdir) > + env_tmpdir = TEMPDIR; > + > + c = strchr(env_tmpdir, '/'); > + /* > + * We force environment variable TMPDIR to be an absolute pathname, > + * which does not make much sense, but it will greatly simplify code in > + * tst_rmdir(). > + */ This looks more like user documentation that anything else. I guess that it would make sense to remove this comment from here and add a note about absolute pathname to doc/user-guide.txt where we describe all the environment variables. > + if (c != env_tmpdir) { > + tst_brkm(TBROK, NULL, "You must specify an absolute " > + "pathname for environment variable TMPDIR"); > + return NULL; > + } Isn't this just overly complicated way how to write: if (env_tmpdir[0] != '/') { tst_brkm(...); ... } > + return env_tmpdir; > +} > + > const char *tst_get_startwd(void) > { > return test_start_work_dir; > @@ -245,31 +267,16 @@ static int rmobj(const char *obj, char **errmsg) > void tst_tmpdir(void) > { > char template[PATH_MAX]; > - char *env_tmpdir; > - char *errmsg, *c; > + const char *env_tmpdir; > + char *errmsg; > > /* > * Create a template for the temporary directory. Use the > * environment variable TMPDIR if it is available, otherwise > * use our default TEMPDIR. > */ > - env_tmpdir = getenv("TMPDIR"); > - if (env_tmpdir) { > - c = strchr(env_tmpdir, '/'); > - /* > - * Now we force environment variable TMPDIR to be an absolute > - * pathname, which dose not make much sense, but it will > - * greatly simplify code in tst_rmdir(). > - */ > - if (c != env_tmpdir) { > - tst_brkm(TBROK, NULL, "You must specify an absolute " > - "pathname for environment variable TMPDIR"); > - return; > - } > - snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID); > - } else { > - snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", TEMPDIR, TCID); > - } > + env_tmpdir = tst_get_tmpdir_root(); > + snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID); > > /* Make the temporary directory in one shot using mkdtemp. */ > if (mkdtemp(template) == NULL) { > diff --git a/testcases/kernel/security/filecaps/filecaps_common.h b/testcases/kernel/security/filecaps/filecaps_common.h > index 920f6ac6a..0f011868e 100644 > --- a/testcases/kernel/security/filecaps/filecaps_common.h > +++ b/testcases/kernel/security/filecaps/filecaps_common.h > @@ -1,5 +1,6 @@ > #include <limits.h> > #include <stdlib.h> > +#include <old_tmpdir.h> > > static char *fifofile; > > @@ -9,10 +10,8 @@ static const char *get_caps_fifo(void) > fifofile = getenv("FIFOFILE"); > > if (!fifofile) { > - const char *tmpdir = getenv("TMPDIR"); > + const char *tmpdir = tst_get_tmpdir_root(); > > - if (!tmpdir) > - tmpdir = "/tmp"; > fifofile = malloc(PATH_MAX); > snprintf(fifofile, PATH_MAX, "%s/caps_fifo", tmpdir); > } > diff --git a/testcases/kernel/syscalls/getcwd/getcwd02.c b/testcases/kernel/syscalls/getcwd/getcwd02.c > index e843a4896..cb111a698 100644 > --- a/testcases/kernel/syscalls/getcwd/getcwd02.c > +++ b/testcases/kernel/syscalls/getcwd/getcwd02.c > @@ -42,28 +42,6 @@ static int dir_exists(const char *dirpath) > return 0; > } > > -static const char *get_tmpdir_path(void) > -{ > - char *tmpdir = "/tmp"; > - > - if (dir_exists(tmpdir)) > - goto done; > - > - /* fallback to $TMPDIR */ > - tmpdir = getenv("TMPDIR"); > - if (!tmpdir) > - tst_brk(TBROK | TERRNO, "Failed to get $TMPDIR"); > - > - if (tmpdir[0] != '/') > - tst_brk(TBROK, "$TMPDIR must be an absolute path"); > - > - if (!dir_exists(tmpdir)) > - tst_brk(TBROK | TERRNO, "TMPDIR '%s' doesn't exist", tmpdir); > - > -done: > - return tmpdir; > -} > - > static void verify_getcwd(unsigned int n) > { > struct t_case *tc = &tcases[n]; > @@ -92,7 +70,10 @@ end: > > static void setup(void) > { > - const char *tmpdir = get_tmpdir_path(); > + const char *tmpdir = tst_get_tmpdir_root(); > + > + if (!dir_exists(tmpdir)) > + tst_brk(TBROK | TERRNO, "TMPDIR '%s' doesn't exist", tmpdir); > > SAFE_CHDIR(tmpdir); > > diff --git a/testcases/open_posix_testsuite/include/tempfile.h b/testcases/open_posix_testsuite/include/tempfile.h > index 0fd27cee3..63e179baf 100644 > --- a/testcases/open_posix_testsuite/include/tempfile.h > +++ b/testcases/open_posix_testsuite/include/tempfile.h > @@ -6,14 +6,8 @@ > #include <stdlib.h> > #include <stdio.h> > #include <limits.h> > +#include <old_tmpdir.h> > > #define PTS_GET_TMP_FILENAME(target, prefix) \ > snprintf(target, sizeof(target), \ > - "%s/" prefix "_pid-%d", pts_get_tmpdir(), getpid()); > - > -static inline const char *pts_get_tmpdir(void) > -{ > - const char *tmpdir_env; > - tmpdir_env = getenv("TMPDIR"); > - return tmpdir_env ? tmpdir_env : "/tmp"; > -} > + "%s/" prefix "_pid-%d", tst_get_tmpdir_root(), getpid()); NACK to this part. The Open Posix Testsuite is not integrated into LTP and does not use the test library. I doubt that this will even compile correctly.
diff --git a/include/old/old_tmpdir.h b/include/old/old_tmpdir.h index 9c61172fd..b1cf79344 100644 --- a/include/old/old_tmpdir.h +++ b/include/old/old_tmpdir.h @@ -45,6 +45,11 @@ void tst_rmdir(void); */ char *tst_get_tmpdir(void); +/* + * Returns path to the test temporary directory root. + */ +const char *tst_get_tmpdir_root(void); + /* * Returns 1 if temp directory was created. */ diff --git a/include/tst_test.h b/include/tst_test.h index a69965b95..26a5ec752 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -362,6 +362,11 @@ void tst_set_max_runtime(int max_runtime); */ char *tst_get_tmpdir(void); +/* + * Returns path to the test temporary directory root. + */ +const char *tst_get_tmpdir_root(void); + /* * Validates exit status of child processes */ diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c index 6642d6be4..50699bc63 100644 --- a/lib/tst_cgroup.c +++ b/lib/tst_cgroup.c @@ -645,10 +645,7 @@ static void cgroup_mount_v2(void) { int ret; char mnt_path[PATH_MAX]; - const char *tmpdir = getenv("TMPDIR"); - - if (!tmpdir) - tmpdir = "/tmp"; + const char *tmpdir = tst_get_tmpdir_root(); sprintf(mnt_path, "%s/%s%s", tmpdir, cgroup_mount_ltp_prefix, cgroup_v2_ltp_mount); @@ -698,10 +695,7 @@ static void cgroup_mount_v1(struct cgroup_ctrl *const ctrl) { char mnt_path[PATH_MAX]; int made_dir = 0; - const char *tmpdir = getenv("TMPDIR"); - - if (!tmpdir) - tmpdir = "/tmp"; + const char *tmpdir = tst_get_tmpdir_root(); if (ctrl->ctrl_indx == CTRL_BLKIO && controllers[CTRL_IO].ctrl_root) { tst_res(TCONF, diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c index 7781f94c3..d4911fa3b 100644 --- a/lib/tst_supported_fs_types.c +++ b/lib/tst_supported_fs_types.c @@ -74,14 +74,11 @@ int tst_fs_in_skiplist(const char *fs_type, const char *const *skiplist) static enum tst_fs_impl has_kernel_support(const char *fs_type) { static int fuse_supported = -1; - const char *tmpdir = getenv("TMPDIR"); + const char *tmpdir = tst_get_tmpdir_root(); char buf[128]; char template[PATH_MAX]; int ret; - if (!tmpdir) - tmpdir = "/tmp"; - snprintf(template, sizeof(template), "%s/mountXXXXXX", tmpdir); if (!mkdtemp(template)) tst_brk(TBROK | TERRNO, "mkdtemp(%s) failed", template); diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c index 6e38ae977..9c90f481a 100644 --- a/lib/tst_tmpdir.c +++ b/lib/tst_tmpdir.c @@ -122,6 +122,28 @@ char *tst_get_tmpdir(void) return ret; } +const char *tst_get_tmpdir_root(void) +{ + char *c; + const char *env_tmpdir = getenv("TMPDIR"); + + if (!env_tmpdir) + env_tmpdir = TEMPDIR; + + c = strchr(env_tmpdir, '/'); + /* + * We force environment variable TMPDIR to be an absolute pathname, + * which does not make much sense, but it will greatly simplify code in + * tst_rmdir(). + */ + if (c != env_tmpdir) { + tst_brkm(TBROK, NULL, "You must specify an absolute " + "pathname for environment variable TMPDIR"); + return NULL; + } + return env_tmpdir; +} + const char *tst_get_startwd(void) { return test_start_work_dir; @@ -245,31 +267,16 @@ static int rmobj(const char *obj, char **errmsg) void tst_tmpdir(void) { char template[PATH_MAX]; - char *env_tmpdir; - char *errmsg, *c; + const char *env_tmpdir; + char *errmsg; /* * Create a template for the temporary directory. Use the * environment variable TMPDIR if it is available, otherwise * use our default TEMPDIR. */ - env_tmpdir = getenv("TMPDIR"); - if (env_tmpdir) { - c = strchr(env_tmpdir, '/'); - /* - * Now we force environment variable TMPDIR to be an absolute - * pathname, which dose not make much sense, but it will - * greatly simplify code in tst_rmdir(). - */ - if (c != env_tmpdir) { - tst_brkm(TBROK, NULL, "You must specify an absolute " - "pathname for environment variable TMPDIR"); - return; - } - snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID); - } else { - snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", TEMPDIR, TCID); - } + env_tmpdir = tst_get_tmpdir_root(); + snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID); /* Make the temporary directory in one shot using mkdtemp. */ if (mkdtemp(template) == NULL) { diff --git a/testcases/kernel/security/filecaps/filecaps_common.h b/testcases/kernel/security/filecaps/filecaps_common.h index 920f6ac6a..0f011868e 100644 --- a/testcases/kernel/security/filecaps/filecaps_common.h +++ b/testcases/kernel/security/filecaps/filecaps_common.h @@ -1,5 +1,6 @@ #include <limits.h> #include <stdlib.h> +#include <old_tmpdir.h> static char *fifofile; @@ -9,10 +10,8 @@ static const char *get_caps_fifo(void) fifofile = getenv("FIFOFILE"); if (!fifofile) { - const char *tmpdir = getenv("TMPDIR"); + const char *tmpdir = tst_get_tmpdir_root(); - if (!tmpdir) - tmpdir = "/tmp"; fifofile = malloc(PATH_MAX); snprintf(fifofile, PATH_MAX, "%s/caps_fifo", tmpdir); } diff --git a/testcases/kernel/syscalls/getcwd/getcwd02.c b/testcases/kernel/syscalls/getcwd/getcwd02.c index e843a4896..cb111a698 100644 --- a/testcases/kernel/syscalls/getcwd/getcwd02.c +++ b/testcases/kernel/syscalls/getcwd/getcwd02.c @@ -42,28 +42,6 @@ static int dir_exists(const char *dirpath) return 0; } -static const char *get_tmpdir_path(void) -{ - char *tmpdir = "/tmp"; - - if (dir_exists(tmpdir)) - goto done; - - /* fallback to $TMPDIR */ - tmpdir = getenv("TMPDIR"); - if (!tmpdir) - tst_brk(TBROK | TERRNO, "Failed to get $TMPDIR"); - - if (tmpdir[0] != '/') - tst_brk(TBROK, "$TMPDIR must be an absolute path"); - - if (!dir_exists(tmpdir)) - tst_brk(TBROK | TERRNO, "TMPDIR '%s' doesn't exist", tmpdir); - -done: - return tmpdir; -} - static void verify_getcwd(unsigned int n) { struct t_case *tc = &tcases[n]; @@ -92,7 +70,10 @@ end: static void setup(void) { - const char *tmpdir = get_tmpdir_path(); + const char *tmpdir = tst_get_tmpdir_root(); + + if (!dir_exists(tmpdir)) + tst_brk(TBROK | TERRNO, "TMPDIR '%s' doesn't exist", tmpdir); SAFE_CHDIR(tmpdir); diff --git a/testcases/open_posix_testsuite/include/tempfile.h b/testcases/open_posix_testsuite/include/tempfile.h index 0fd27cee3..63e179baf 100644 --- a/testcases/open_posix_testsuite/include/tempfile.h +++ b/testcases/open_posix_testsuite/include/tempfile.h @@ -6,14 +6,8 @@ #include <stdlib.h> #include <stdio.h> #include <limits.h> +#include <old_tmpdir.h> #define PTS_GET_TMP_FILENAME(target, prefix) \ snprintf(target, sizeof(target), \ - "%s/" prefix "_pid-%d", pts_get_tmpdir(), getpid()); - -static inline const char *pts_get_tmpdir(void) -{ - const char *tmpdir_env; - tmpdir_env = getenv("TMPDIR"); - return tmpdir_env ? tmpdir_env : "/tmp"; -} + "%s/" prefix "_pid-%d", tst_get_tmpdir_root(), getpid());
Signed-off-by: Edward Liaw <edliaw@google.com> --- include/old/old_tmpdir.h | 5 +++ include/tst_test.h | 5 +++ lib/tst_cgroup.c | 10 +---- lib/tst_supported_fs_types.c | 5 +-- lib/tst_tmpdir.c | 45 +++++++++++-------- .../security/filecaps/filecaps_common.h | 5 +-- testcases/kernel/syscalls/getcwd/getcwd02.c | 27 ++--------- .../open_posix_testsuite/include/tempfile.h | 10 +---- 8 files changed, 47 insertions(+), 65 deletions(-)