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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240803101625.GH29127@1wt.eu>
Date: Sat, 3 Aug 2024 12:16:25 +0200
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Shuah Khan <skhan@...uxfoundation.org>, Shuah Khan <shuah@...nel.org>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 1/2] tools/nolibc: add support for [v]sscanf()

On Fri, Aug 02, 2024 at 05:48:13PM +0200, Thomas Weißschuh wrote:
> > With all these libc functionality added, it isn't nolibc looks like :)
> 
> Well :-)
> 
> The main motivation is to provide kselftests compatibility.
> Maybe Willy disagrees.

No no I'm perfectly fine with adding the functions that developers use
or need to write their test or init tools. I don't have any strong
opinion on scanf(). Just like strtok(), I stopped using it 25 years ago
when I noticed that it never survives code evolutions, lacks a lot of
flexibility and is often strongly tied to your types (more than printf
where you can cast). But I perfectly understand that others are used to
it and would appreciate to have it, for example if it helps with command
line arguments.

> > > +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.
> 
> I tried that and didn't find a way.
> Any pointers are welcome.

I think it would be difficult by nature of varargs.

However, since you grouped some expressions, maybe a one-liner comment
between each scanf() to explain the intent of the test would make it
easier to follow. E.g:
    /* test multiple naked numbers */
    ...
    /* test numbers delimited with a character */
    ...
    /* test multiple integer types at once */

etc. This allows the reviewer to more easly re-focus on the test they
were reading.

Willy