Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions

[Rasmus Villemoes]

On 21/02/2020 17.33, Uwe Kleine-König wrote:
> On Fri, Feb 21, 2020 at 05:27:49PM +0200, Andy Shevchenko wrote:
>> On Fri, Feb 21, 2020 at 4:54 PM Uwe Kleine-König
>> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>>> On Fri, Feb 21, 2020 at 10:57:23AM +0200, Andy Shevchenko wrote:
>>>> The commit 885e68e8b7b1 ("kernel.h: update comment about simple_strto<foo>()
>>>> functions") updated a comment regard to simple_strto<foo>() functions, but
>>>> missed similar change in the vsprintf.c module.
>>>>
>>>> Update comments in vsprintf.c as well for simple_strto<foo>() functions.
>>
>> ...
>>
>>>> - * This function is obsolete. Please use kstrtoull instead.
>>>> + * This function has caveats. Please use kstrtoull instead.
>>
>>> I wonder if we instead want to create a set of functions that is
>>> versatile enough to cover kstrtoull and simple_strtoull. i.e. fix the
>>> rounding problems (that are the caveats, right?) and as calling
>>> convention use an errorvalued int return + an output-parameter of the
>>> corresponding type.
>>
>> It wouldn't be possible to apply same rules to both. They both are
>> part of existing ABI.
> 
> The idea is to creat a sane set of functions, then convert all users to
> the sane one and only then strip the strange functions away. (Userspace)
> ABI isn't affected, is it?

There are lots of in-tree users of all these interfaces, converting them
all is never going to happen. And yes, there are also kstrtox_user
variants which are definitely part of ABI (more or less the whole reason
kstrox accepts a single trailing newline but is otherwise rather strict
is so it can parse stuff that is echo'd to a sysfs/procfs/... file).

But, one can at least try to unify the underlying integer parsing, and
provide knobs for users (meaning other parts of the kernel) to decide
how lax or strict to be in a given situation. Based on an idea from
Alexey Dobriyan of creating a single type-generic parser, I did that a
couple of years ago. It rebased pretty cleanly, so if anyone wants to
take a look:

https://github.com/Villemoes/linux/tree/parse-integer

There's certainly more to do, but just doing all kstrtox() in terms of
parse_integer() seems to reduce vmlinux size. Next steps would be the
simple_strtox family, which more or less just need PARSE_INTEGER_WRAP
AFAICT.

And stuff like strtoul_lenient in kernel/sysctl.c is an example of a
caller that wants to specify the laxness of the parsing (no overflow
allowed, so simple_* doesn't cut it, but we're parsing a string where we
might want to continue after the current number, so kstrtox is too
strict in terms of what trailers are allowed).

Rasmus