[1/4] lib/tst_tmpdir: Normalize user defined TMPDIR

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]).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_tmpdir.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Hi,

> 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]).

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  lib/tst_tmpdir.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)

> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index b73b5c66f..bc9351251 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -124,7 +124,8 @@ char *tst_get_tmpdir(void)

>  const char *tst_get_tmpdir_root(void)
>  {
> -	const char *env_tmpdir = getenv("TMPDIR");
> +	char *p, *env_tmpdir = getenv("TMPDIR");
> +	int fixed = 0;

>  	if (!env_tmpdir)
>  		env_tmpdir = TEMPDIR;
> @@ -134,6 +135,23 @@ const char *tst_get_tmpdir_root(void)
>  				"pathname for environment variable TMPDIR");
>  		return NULL;
>  	}
> +
> +	if (env_tmpdir[strlen(env_tmpdir)-1] == '/') {
> +		env_tmpdir[strlen(env_tmpdir)-1] = '\0';
> +		fixed = 1;
> +	}
Actually, this above supposed to be after removing double slash (in case people
are crazy to put /// or more in the end, see patch bellow.

I'm sorry for the noise.

Kind regards,
Petr

> +
> +	while ((p = strstr(env_tmpdir, "//")) != NULL) {
> +		memmove(p, p + 1, strlen(env_tmpdir) - (p - env_tmpdir));
> +		fixed = 1;
> +	}
> +
> +	if (fixed) {
> +		tst_resm(TINFO, "WARNING: Remove double or trailing slashes from TMPDIR,"
> +			 " please fix your setup to: TMPDIR='%s'",
> +			 env_tmpdir);
> +	}
> +
>  	return env_tmpdir;
>  }

diff --git lib/newlib_tests/tst_tmpdir.c lib/newlib_tests/tst_tmpdir.c
index 008542808..c34430fa6 100644
--- lib/newlib_tests/tst_tmpdir.c
+++ lib/newlib_tests/tst_tmpdir.c
@@ -18,7 +18,8 @@ static struct tcase {
 } tcases[] = {
 	{NULL, TEMPDIR, "default value"},
 	{"/tmp/", "/tmp", "removing trailing slash"},
-	{"//var///tmp", "/var/tmp", "removing duplicate slashes"},
+	{"/var//tmp", "/var/tmp", "removing duplicate slashes"},
+	{"//var///tmp///", "/var/tmp", "removing too many slashes"},
 };
 
 static void do_test(unsigned int nr)

Hi!
> +	if (env_tmpdir[strlen(env_tmpdir)-1] == '/') {
> +		env_tmpdir[strlen(env_tmpdir)-1] = '\0';
> +		fixed = 1;
> +	}
> +
> +	while ((p = strstr(env_tmpdir, "//")) != NULL) {
> +		memmove(p, p + 1, strlen(env_tmpdir) - (p - env_tmpdir));
> +		fixed = 1;
> +	}
> +
> +	if (fixed) {
> +		tst_resm(TINFO, "WARNING: Remove double or trailing slashes from TMPDIR,"
> +			 " please fix your setup to: TMPDIR='%s'",
> +			 env_tmpdir);
> +	}
> +

This whole thing can be just a single loop (beware untested):

	size_t s = 0, d = 0;
	char prev_c = 0;

	for (;;) {
		switch (env_tmpdir[s]) {
		case '/':
			if (prev_c != '/')
				d++;
			s++;
		break;
		case '\0':
			if (d && prev_c == '/')
				env_tmpdir[d-1] = '\0';
			break;
		break;
		default:
			env_tmpdir[d++] = env_tmpdir[s++];
		}
	}

Hi!
> +	if (fixed) {
> +		tst_resm(TINFO, "WARNING: Remove double or trailing slashes from TMPDIR,"
> +			 " please fix your setup to: TMPDIR='%s'",
> +			 env_tmpdir);
> +	}

Also why do we bother with a warning when we fixed the path. I would
either fix it and go on or fail the test if it's detected.

> Hi!
> > +	if (env_tmpdir[strlen(env_tmpdir)-1] == '/') {
> > +		env_tmpdir[strlen(env_tmpdir)-1] = '\0';
> > +		fixed = 1;
> > +	}
> > +
> > +	while ((p = strstr(env_tmpdir, "//")) != NULL) {
> > +		memmove(p, p + 1, strlen(env_tmpdir) - (p - env_tmpdir));
> > +		fixed = 1;
> > +	}
> > +
> > +	if (fixed) {
> > +		tst_resm(TINFO, "WARNING: Remove double or trailing slashes from TMPDIR,"
> > +			 " please fix your setup to: TMPDIR='%s'",
> > +			 env_tmpdir);
> > +	}
> > +

> This whole thing can be just a single loop (beware untested):

> 	size_t s = 0, d = 0;
> 	char prev_c = 0;

> 	for (;;) {
> 		switch (env_tmpdir[s]) {

NOTE you never set prev_c, it would have to be:

	if (s)
		prev_c = env_tmpdir[s-1];

> 		case '/':
> 			if (prev_c != '/')
> 				d++;
> 			s++;
> 		break;
> 		case '\0':
> 			if (d && prev_c == '/')
> 				env_tmpdir[d-1] = '\0';
> 			break;

And also you don't break for loop, because the break above is for case, goto or
return would have to be used).

> 		break;
> 		default:
> 			env_tmpdir[d++] = env_tmpdir[s++];
> 		}
> 	}

Also if TMPDIR is not set, env_tmpdir = TEMPDIR; will lead to segfault.
strdup() will need to be used:

	if (!env_tmpdir)
		env_tmpdir = strdup(TEMPDIR);

But even with these changes I was not able to find why / is eaten:


	char *env_tmpdir = getenv("TMPDIR");
	size_t s = 0, d = 0;
	char prev_c = 0;

	if (!env_tmpdir)
		env_tmpdir = strdup(TEMPDIR);

	for (;;) {
		if (s)
			prev_c = env_tmpdir[s-1];

		switch (env_tmpdir[s]) {
		case '/':
			if (prev_c != '/')
				d++;
			s++;
		break;
		case '\0':
			if (d && prev_c == '/')
				env_tmpdir[d-1] = '\0';
			goto end;
		break;
		default:
			env_tmpdir[d++] = env_tmpdir[s++];
		}
	}

end:
	return env_tmpdir;
}


So how about using for loop? It's not that compact, but maybe easily readable.

const char *tst_get_tmpdir_root(void)
{
	char *env_tmpdir = getenv("TMPDIR");
	char prev_c = 0;
	size_t k = 0;

	if (!env_tmpdir)
		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;
}

Kind regards,
Petr