[LTP] [PATCH v2 0/3] Remove double or trailing slashes in TMPDIR in C API

[LTP] [PATCH v2 0/3] Remove double or trailing slashes in TMPDIR in C API

[Petr Vorel]

Changes v1->v2:
* Change algorithm in lib/tst_tmpdir.c (different than the suggested one
  by Cyril in v1).
* Remove warnings about wrong TMPDIR.

Petr Vorel (3):
  lib/tst_tmpdir: Normalize user defined TMPDIR
  lib: Add test for tst_tmpdir
  lib: Improve doc related to $TMPDIR default value

 include/tst_defaults.h        |  3 ++-
 lib/newlib_tests/runtest.sh   |  2 +-
 lib/newlib_tests/tst_tmpdir.c | 49 +++++++++++++++++++++++++++++++++++
 lib/tst_tmpdir.c              | 20 ++++++++++++--
 testcases/lib/tst_test.sh     |  2 ++
 5 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 lib/newlib_tests/tst_tmpdir.c

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/3] lib/tst_tmpdir: Normalize user defined TMPDIR

[Petr Vorel]

Follow the changes to shell API 273c49793 ("tst_test.sh: Remove possible
double/trailing slashes from TMPDIR") and remove: 1) trailing slash
2) double slashes.

This is needed, because some tests compare file path of files which are
in TMPDIR with strcmp() or and extra slashes break it (e.g. chdir01A,
ioctl_loop0[12], mount0[67]).

Co-developed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_tmpdir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index b73b5c66f..b6e51ba0a 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -124,16 +124,32 @@ char *tst_get_tmpdir(void)
 
 const char *tst_get_tmpdir_root(void)
 {
-	const char *env_tmpdir = getenv("TMPDIR");
+	char *env_tmpdir = getenv("TMPDIR");
+	char prev_c = 0;
+	size_t k = 0;
 
 	if (!env_tmpdir)
-		env_tmpdir = TEMPDIR;
+		env_tmpdir = strdup(TEMPDIR);
 
 	if (env_tmpdir[0] != '/') {
 		tst_brkm(TBROK, NULL, "You must specify an absolute "
 				"pathname for environment variable TMPDIR");
 		return NULL;
 	}
+
+	for (int i = 0; env_tmpdir[i] != '\0'; i++) {
+		if (i)
+			prev_c = env_tmpdir[i-1];
+
+        if (env_tmpdir[i] != '/' || prev_c != '/')
+            env_tmpdir[k++] = env_tmpdir[i];
+	}
+
+	env_tmpdir[k] = '\0';
+
+	if (env_tmpdir[k-1] == '/')
+		env_tmpdir[k-1] = '\0';
+
 	return env_tmpdir;
 }
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/3] lib: Add test for tst_tmpdir

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/newlib_tests/runtest.sh   |  2 +-
 lib/newlib_tests/tst_tmpdir.c | 49 +++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 lib/newlib_tests/tst_tmpdir.c

diff --git a/lib/newlib_tests/runtest.sh b/lib/newlib_tests/runtest.sh
index 8f2a497b1..7bdc870bc 100755
--- a/lib/newlib_tests/runtest.sh
+++ b/lib/newlib_tests/runtest.sh
@@ -5,7 +5,7 @@ LTP_C_API_TESTS="${LTP_C_API_TESTS:-test05 test07 test09 test15 test_runtime01
 tst_needs_cmds01 tst_needs_cmds02 tst_needs_cmds03 tst_needs_cmds06
 tst_needs_cmds07 tst_bool_expr test_exec test_timer tst_res_hexd tst_strstatus
 tst_fuzzy_sync03 test_zero_hugepage.sh test_kconfig.sh test_kconfig03
-test_children_cleanup.sh}"
+test_children_cleanup.sh tst_tmpdir}"
 
 LTP_SHELL_API_TESTS="${LTP_SHELL_API_TESTS:-shell/tst_check_driver.sh
 shell/tst_check_kconfig0[1-5].sh shell/tst_errexit.sh shell/net/*.sh}"
diff --git a/lib/newlib_tests/tst_tmpdir.c b/lib/newlib_tests/tst_tmpdir.c
new file mode 100644
index 000000000..c34430fa6
--- /dev/null
+++ b/lib/newlib_tests/tst_tmpdir.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Petr Vorel <pvorel@suse.cz>
+ */
+
+/*
+ * Tests TMPDIR variable cleanup.
+ */
+
+#include <stdlib.h>
+#include "tst_defaults.h"
+#include "tst_test.h"
+
+static struct tcase {
+	const char *orig;
+	const char *fixed;
+	const char *desc;
+} tcases[] = {
+	{NULL, TEMPDIR, "default value"},
+	{"/tmp/", "/tmp", "removing trailing slash"},
+	{"/var//tmp", "/var/tmp", "removing duplicate slashes"},
+	{"//var///tmp///", "/var/tmp", "removing too many slashes"},
+};
+
+static void do_test(unsigned int nr)
+{
+	struct tcase *tc = &tcases[nr];
+	const char *env_tmpdir;
+
+	tst_res(TINFO, "Testing TMPDIR='%s' (%s)", tc->orig, tc->desc);
+
+	if (tc->orig)
+		SAFE_SETENV("TMPDIR", tc->orig, 1);
+
+	env_tmpdir = tst_get_tmpdir_root();
+
+	if (!env_tmpdir)
+		tst_brk(TBROK, "Failed to get TMPDIR");
+
+	if (!strcmp(tc->fixed, env_tmpdir))
+		tst_res(TPASS, "TMPDIR '%s' is '%s'", env_tmpdir, tc->fixed);
+	else
+		tst_res(TFAIL, "TMPDIR '%s' should be '%s'", env_tmpdir, tc->fixed);
+}
+
+static struct tst_test test = {
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(tcases),
+};
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 3/3] lib: Improve doc related to $TMPDIR default value

[Petr Vorel]

Link C and shell API related parts to $TMPDIR.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/tst_defaults.h    | 3 ++-
 testcases/lib/tst_test.sh | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/tst_defaults.h b/include/tst_defaults.h
index 083427b7e..abe1e6956 100644
--- a/include/tst_defaults.h
+++ b/include/tst_defaults.h
@@ -8,8 +8,9 @@
 
 /*
  * This is the default temporary directory used by tst_tmpdir().
+ * used when TMPDIR env variable is not set.
  *
- * This is used when TMPDIR env variable is not set.
+ * If changed, update also shell API in testcases/lib/tst_test.sh.
  */
 #define TEMPDIR	"/tmp"
 
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 0d2fccb95..b595bb5ea 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -734,6 +734,8 @@ tst_run()
 
 	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
 		if [ -z "$TMPDIR" ]; then
+			# Default value if TMPDIR not set.
+			# If changed, update also C API in tst_defaults.h.
 			export TMPDIR="/tmp"
 		fi
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/3] lib/tst_tmpdir: Normalize user defined TMPDIR

[Martin Doucha]

Hi,
I don't like the approach of directly modifying the process environment. 
Also, if TMPDIR variable is not set, the newly added strdup() will lead 
to a memory leak because the function is called multiple times.

I recommend adding a static pointer that'll hold the cleaned up path and 
the string cleanup will run only once on the first call. Then the 
pointer will be free()d during library cleanup.

On 25. 03. 24 12:50, Petr Vorel wrote:
> Follow the changes to shell API 273c49793 ("tst_test.sh: Remove possible
> double/trailing slashes from TMPDIR") and remove: 1) trailing slash
> 2) double slashes.
> 
> This is needed, because some tests compare file path of files which are
> in TMPDIR with strcmp() or and extra slashes break it (e.g. chdir01A,
> ioctl_loop0[12], mount0[67]).
> 
> Co-developed-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   lib/tst_tmpdir.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index b73b5c66f..b6e51ba0a 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -124,16 +124,32 @@ char *tst_get_tmpdir(void)
>   
>   const char *tst_get_tmpdir_root(void)
>   {
> -	const char *env_tmpdir = getenv("TMPDIR");
> +	char *env_tmpdir = getenv("TMPDIR");
> +	char prev_c = 0;
> +	size_t k = 0;
>   
>   	if (!env_tmpdir)
> -		env_tmpdir = TEMPDIR;
> +		env_tmpdir = strdup(TEMPDIR);
>   
>   	if (env_tmpdir[0] != '/') {
>   		tst_brkm(TBROK, NULL, "You must specify an absolute "
>   				"pathname for environment variable TMPDIR");
>   		return NULL;
>   	}
> +
> +	for (int i = 0; env_tmpdir[i] != '\0'; i++) {
> +		if (i)
> +			prev_c = env_tmpdir[i-1];
> +
> +        if (env_tmpdir[i] != '/' || prev_c != '/')
> +            env_tmpdir[k++] = env_tmpdir[i];
> +	}
> +
> +	env_tmpdir[k] = '\0';
> +
> +	if (env_tmpdir[k-1] == '/')
> +		env_tmpdir[k-1] = '\0';
> +
>   	return env_tmpdir;
>   }
>   

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/3] lib/tst_tmpdir: Normalize user defined TMPDIR

[Petr Vorel]

Hi Martin,

> Hi,
> I don't like the approach of directly modifying the process environment.
> Also, if TMPDIR variable is not set, the newly added strdup() will lead to a
> memory leak because the function is called multiple times.

> I recommend adding a static pointer that'll hold the cleaned up path and the
> string cleanup will run only once on the first call. Then the pointer will
> be free()d during library cleanup.

Good, point, thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp