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

[Petr Vorel <pvorel@suse.cz>]

> 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

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