Re: [LTP] [PATCH v1] Rewrite confstr01.c test using new LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

thanks for looking into this.

I was looking into the test in the past. The reason I haven't sent it is that
some of the definitions are in the legacy definition:
https://pubs.opengroup.org/onlinepubs/000095399/functions/confstr.html

in newer it's not:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html

...
>  static struct test_case_t {
>  	int value;
>  	char *name;
>  } test_cases[] = {
> -	{_CS_PATH, "PATH"},
> -	{_CS_XBS5_ILP32_OFF32_CFLAGS, "XBS5_ILP32_OFF32_CFLAGS"},
> -	{_CS_XBS5_ILP32_OFF32_LDFLAGS, "XBS5_ILP32_OFF32_LDFLAGS"},
> -	{_CS_XBS5_ILP32_OFF32_LIBS, "XBS5_ILP32_OFF32_LIBS"},
> -	{_CS_XBS5_ILP32_OFF32_LINTFLAGS, "XBS5_ILP32_OFF32_LINTFLAGS"},
> -	{_CS_XBS5_ILP32_OFFBIG_CFLAGS, "XBS5_ILP32_OFFBIG_CFLAGS"},
> -	{_CS_XBS5_ILP32_OFFBIG_LDFLAGS, "XBS5_ILP32_OFFBIG_LDFLAGS"},
> -	{_CS_XBS5_ILP32_OFFBIG_LIBS, "XBS5_ILP32_OFFBIG_LIBS"},
> -	{_CS_XBS5_ILP32_OFFBIG_LINTFLAGS, "XBS5_ILP32_OFFBIG_LINTFLAGS"},
> -	{_CS_XBS5_LP64_OFF64_CFLAGS, "XBS5_LP64_OFF64_CFLAGS"},
> -	{_CS_XBS5_LP64_OFF64_LDFLAGS, "XBS5_LP64_OFF64_LDFLAGS"},
> -	{_CS_XBS5_LP64_OFF64_LIBS, "XBS5_LP64_OFF64_LIBS"},
> -	{_CS_XBS5_LP64_OFF64_LINTFLAGS, "XBS5_LP64_OFF64_LINTFLAGS"},
> -	{_CS_XBS5_LPBIG_OFFBIG_CFLAGS, "XBS5_LPBIG_OFFBIG_CFLAGS"},
> -	{_CS_XBS5_LPBIG_OFFBIG_LDFLAGS, "XBS5_LPBIG_OFFBIG_LDFLAGS"},
> -	{_CS_XBS5_LPBIG_OFFBIG_LIBS, "XBS5_LPBIG_OFFBIG_LIBS"},
> -	{_CS_XBS5_LPBIG_OFFBIG_LINTFLAGS, "XBS5_LPBIG_OFFBIG_LINTFLAGS"},
> -	{_CS_GNU_LIBC_VERSION, "GNU_LIBC_VERSION"},
> -	{_CS_GNU_LIBPTHREAD_VERSION, "GNU_LIBPTHREAD_VERSION"},
> +	{ _CS_PATH, "PATH" },
> +	{ _CS_GNU_LIBC_VERSION, "GNU_LIBC_VERSION" },
man CONFSTR(3) shows
 _CS_GNU_LIBC_VERSION (GNU C library only; since glibc 2.3.2)

And indeed it fails on musl:
confstr01.c:102: TFAIL: confstr: GNU_LIBC_VERSION, EINVAL

But musl defines in include/unistd.h:
#define _CS_GNU_LIBC_VERSION    2

to allow to compile but in src/conf/confstr.c is check which causes EINVAL:

https://git.etalabs.net/cgit/musl/tree/src/conf/confstr.c
size_t confstr(int name, char *buf, size_t len)
{
	const char *s = "";
	if (!name) {
		s = "/bin:/usr/bin";
	} else if ((name&~4U)!=1 && name-_CS_POSIX_V6_ILP32_OFF32_CFLAGS>33U) {
		errno = EINVAL;
		return 0;
	}
	// snprintf is overkill but avoid wasting code size to implement
	// this completely useless function and its truncation semantics
	return snprintf(buf, len, "%s", s) + 1;
}

IMHO this one should be wrapped with #ifdef __GLIBC__.
Also not sure how about uclibc (it defines __GLIBC__ and it has compatible
headers, but sometimes things aren't implemented). It defines
_CS_GNU_LIBC_VERSION as "GNU_LIBC_VERSION", but code which is in posix/confstr.c
is missing. I might force myself to check it after work time.

...

> +	TEST(confstr(test_cases[i].value, NULL, (size_t)0));

> +	if (TST_RET) {
> +		len = TST_RET;
> +		buf = SAFE_MALLOC(len);

> +		TEST(confstr(test_cases[i].value, buf, len));

> +		if (buf[len - 1] != '\0') {
> +			tst_brk(TFAIL, "confstr: %s, %s", test_cases[i].name,
> +				tst_strerrno(TST_ERR));
> +		} else {
> +			tst_res(TPASS, "confstr %s = '%s'", test_cases[i].name, buf);
>  		}
> +	} else {
> +		tst_brk(TFAIL, "confstr: %s, %s", test_cases[i].name, tst_strerrno(TST_ERR));
> +	}

Again, to use some TST_EXP_*() macros?
Maybe:

	TST_EXP_POSITIVE(confstr(test_cases[i].value, NULL, (size_t)0));

	if (!TST_PASS)
		return;

	len = TST_RET;
	buf = SAFE_MALLOC(len);

	TEST(confstr(test_cases[i].value, buf, len));

	if (buf[len - 1] != '\0') {
		tst_brk(TFAIL, "confstr: %s, %s", test_cases[i].name,
			tst_strerrno(TST_ERR));
	} else {
		tst_res(TPASS, "confstr %s = '%s'", test_cases[i].name, buf);
	}

	free(buf);

=> code is simpler and works also on musl.

Kind regards,
Petr

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