Re: [PATCH 1/2] tools/nolibc: add support for [v]sscanf()

[Shuah Khan <skhan@linuxfoundation.org>]

On 7/31/24 12:32, Thomas Weißschuh wrote:
> The implementation is limited and only supports numeric arguments.

I would like to see more information in here. Why is this needed
etc. etc.

> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>   tools/include/nolibc/stdio.h                 | 93 ++++++++++++++++++++++++++++
>   tools/testing/selftests/nolibc/nolibc-test.c | 59 ++++++++++++++++++
>   2 files changed, 152 insertions(+)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index c968dbbc4ef8..d63c45c06d8e 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -348,6 +348,99 @@ int printf(const char *fmt, ...)
>   	return ret;
>   }
>   
> +static __attribute__((unused))
> +int vsscanf(const char *str, const char *format, va_list args)

Is there a reason why you didn't use the same code in lib/vsprintf.c?
You could simply duplicate the code here?

With all these libc functionality added, it isn't nolibc looks like :)

> +{
> +	uintmax_t uval;
> +	intmax_t ival;
> +	int base;
> +	char *endptr;
> +	int matches;
> +	int lpref;
> +
> +	matches = 0;
> +
> +	while (1) {
> +		if (*format == '%') {
> +			lpref = 0;
> +			format++;
> +
> +			if (*format == 'l') {
> +				lpref = 1;
> +				format++;
> +				if (*format == 'l') {
> +					lpref = 2;
> +					format++;
> +				}
> +			}
> +
> +			if (*format == '%') {
> +				if ('%' != *str)
> +					goto done;
> +				str++;
> +				format++;
> +				continue;
> +			} else if (*format == 'd') {
> +				ival = strtoll(str, &endptr, 10);
> +				if (lpref == 0)
> +					*va_arg(args, int *) = ival;
> +				else if (lpref == 1)
> +					*va_arg(args, long *) = ival;
> +				else if (lpref == 2)
> +					*va_arg(args, long long *) = ival;
> +			} else if (*format == 'u' || *format == 'x' || *format == 'X') {
> +				base = *format == 'u' ? 10 : 16;
> +				uval = strtoull(str, &endptr, base);
> +				if (lpref == 0)
> +					*va_arg(args, unsigned int *) = uval;
> +				else if (lpref == 1)
> +					*va_arg(args, unsigned long *) = uval;
> +				else if (lpref == 2)
> +					*va_arg(args, unsigned long long *) = uval;
> +			} else if (*format == 'p') {
> +				*va_arg(args, void **) = (void *)strtoul(str, &endptr, 16);
> +			} else {
> +				SET_ERRNO(EILSEQ);
> +				goto done;
> +			}
> +
> +			format++;
> +			str = endptr;
> +			matches++;
> +
> +		} else if (*format == '\0') {
> +			goto done;
> +		} else if (isspace(*format)) {
> +			while (isspace(*format))
> +				format++;
> +			while (isspace(*str))
> +				str++;
> +		} else if (*format == *str) {
> +			format++;
> +			str++;
> +		} else {
> +			if (!matches)
> +				matches = EOF;
> +			goto done;
> +		}
> +	}
> +
> +done:
> +	return matches;
> +}
> +
> +static __attribute__((unused, format(scanf, 2, 3)))
> +int sscanf(const char *str, const char *format, ...)
> +{
> +	va_list args;
> +	int ret;
> +
> +	va_start(args, format);
> +	ret = vsscanf(str, format, args);
> +	va_end(args);
> +	return ret;
> +}
> +
>   static __attribute__((unused))
>   void perror(const char *msg)
>   {
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 093d0512f4c5..addbceb0b276 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -1277,6 +1277,64 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm
>   	return ret;
>   }
>   
> +static int test_scanf(void)
> +{
> +	unsigned long long ull;
> +	unsigned long ul;
> +	unsigned int u;
> +	long long ll;
> +	long l;
> +	void *p;
> +	int i;
> +
> +	if (sscanf("", "foo") != EOF)
> +		return 1;
> +
> +	if (sscanf("foo", "foo") != 0)
> +		return 2;
> +
> +	if (sscanf("123", "%d", &i) != 1)
> +		return 3;
> +
> +	if (i != 123)
> +		return 4;
> +
> +	if (sscanf("a123b456c0x90", "a%db%uc%p", &i, &u, &p) != 3)
> +		return 5;
> +
> +	if (i != 123)
> +		return 6;
> +
> +	if (u != 456)
> +		return 7;
> +
> +	if (p != (void *)0x90)
> +		return 8;
> +
> +	if (sscanf("a    b1", "a b%d", &i) != 1)
> +		return 9;
> +
> +	if (i != 1)
> +		return 10;
> +
> +	if (sscanf("a%1", "a%%%d", &i) != 1)
> +		return 11;
> +
> +	if (i != 1)
> +		return 12;
> +
> +	if (sscanf("1|2|3|4|5|6",
> +		   "%d|%ld|%lld|%u|%lu|%llu",
> +		   &i, &l, &ll, &u, &ul, &ull) != 6)
> +		return 13;
> +
> +	if (i != 1 || l != 2 || ll != 3 ||
> +	    u != 4 || ul != 5 || ull != 6)
> +		return 14;
> +
> +	return 0;

Can we simplify this code? It is hard to read code with too
many conditions. Maybe defining an array test conditions
instead of a series ifs.

> +}
> +
>   static int run_vfprintf(int min, int max)
>   {
>   	int test;
> @@ -1298,6 +1356,7 @@ static int run_vfprintf(int min, int max)
>   		CASE_TEST(char);         EXPECT_VFPRINTF(1, "c", "%c", 'c'); break;
>   		CASE_TEST(hex);          EXPECT_VFPRINTF(1, "f", "%x", 0xf); break;
>   		CASE_TEST(pointer);      EXPECT_VFPRINTF(3, "0x1", "%p", (void *) 0x1); break;
> +		CASE_TEST(scanf);        EXPECT_ZR(1, test_scanf()); break;
>   		case __LINE__:
>   			return ret; /* must be last */
>   		/* note: do not set any defaults so as to permit holes above */
> 

thanks,
-- Shuah