[LTP] [PATCH v1] tst_tmpdir: add TST_GET_TMPDIR_ROOT() to get the TMPDIR env var
Cyril Hrubis
chrubis@suse.cz
Wed Nov 2 14:07:45 CET 2022
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.
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list