Re: [PATCH v1] kernel.h: Update comment about simple_strto<foo>() functions

[Miguel Ojeda]

On Tue, Feb 27, 2018 at 12:40 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> On Mon, 2018-02-26 at 23:31 +0100, Miguel Ojeda wrote:
>> On Mon, Feb 26, 2018 at 6:55 PM, Andy Shevchenko
>> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>> > There were discussions in the past about use cases for
>> > simple_strto<foo>() functions and in some rare cases they have a
>> > benefit
>> > on kstrto<foo>() ones.
>> >
>> > Update a comment to reduce confusing about special use cases.
>> >
>> > Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx>
>>
>> I am not sure we should just go back to the old ones, though.
>
> I didn't tell that we should.
> The niche of kstrto*() and simple_strto*() is different.

I meant that I am not sure that we should just re-allow the old ones
(i.e. I didn't mean that you meant to remove the kstrto*() ones :-).

>
>>  Maybe it
>> is better to create a new set of kstrto*_inplace() or some other name,
>> safer than the old ones and following kstrto*()'s interface regarding
>> returned errors, overflow checking, etc. There are two variations that
>> can be useful:
>>
>>   * A strict version taking a (start, end) range or (start, size) pair
>> which contains the number to be converted. If there is any problem
>> parsing it (e.g. invalid characters, extra characters, ...), fail.
>>
>>   * A less strict version taking an extra end pointer (or size
>> parameter) which is not allowed to be surpassed, and any non-digit
>> character means successful stop.
>
> Send a patch, we will discuss that for sure.
>
>> The old behavior (simple_*()) can still be achieved (almost) with the
>> second version with an "infinite" end pointer if one really needs it.
>
>> In any case, if you want to go forward with the old ones, we would
>> also have to change the comments inside lib/vsprintf.c and possibly
>> checkpatch :-)
>
> Feel free to amend.
>
> I actually didn't get your position here. You rather going to keep ugly
> code in your subsystem because of "official" comment than do it in more
> cleaner, but old fashion way.

I am not going to keep ugly code (why would you say so? It is not even
accepted yet); but I am not adding calls to deprecated functions
either. If those functions shouldn't be deprecated, fine; but *that*
is what should be discussed/changed. Adding new calls to functions
that were deprecated years ago (and currently are in HEAD) does not
help anybody.

>
> Btw, you can still weakly (based on power of base) detect an overflow by
> checking a returned pointer from simple_strto*().

Sure, but fixing that interface is precisely one of the reasons
simple_strto*() were deprecated to begin with.

Cheers,
Miguel

>
> --
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Intel Finland Oy