* [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
@ 2020-02-21 8:57 Andy Shevchenko
2020-02-21 13:08 ` Petr Mladek
2020-02-21 14:51 ` Uwe Kleine-König
0 siblings, 2 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-21 8:57 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Sergey Senozhatsky, linux-kernel,
Rasmus Villemoes
Cc: Andy Shevchenko, Uwe Kleine-König
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.
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
lib/vsprintf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7c488a1ce318..d5641a217685 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -58,7 +58,7 @@
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*
- * This function is obsolete. Please use kstrtoull instead.
+ * This function has caveats. Please use kstrtoull instead.
*/
unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
@@ -83,7 +83,7 @@ EXPORT_SYMBOL(simple_strtoull);
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*
- * This function is obsolete. Please use kstrtoul instead.
+ * This function has caveats. Please use kstrtoul instead.
*/
unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base)
{
@@ -97,7 +97,7 @@ EXPORT_SYMBOL(simple_strtoul);
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*
- * This function is obsolete. Please use kstrtol instead.
+ * This function has caveats. Please use kstrtol instead.
*/
long simple_strtol(const char *cp, char **endp, unsigned int base)
{
@@ -114,7 +114,7 @@ EXPORT_SYMBOL(simple_strtol);
* @endp: A pointer to the end of the parsed string will be placed here
* @base: The number base to use
*
- * This function is obsolete. Please use kstrtoll instead.
+ * This function has caveats. Please use kstrtoll instead.
*/
long long simple_strtoll(const char *cp, char **endp, unsigned int base)
{
--
2.25.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-21 8:57 [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions Andy Shevchenko
@ 2020-02-21 13:08 ` Petr Mladek
2020-02-21 14:51 ` Uwe Kleine-König
1 sibling, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2020-02-21 13:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Steven Rostedt, Sergey Senozhatsky, linux-kernel,
Rasmus Villemoes, Uwe Kleine-König
On Fri 2020-02-21 10:57:23, 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.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Looks good to me.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-21 8:57 [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions Andy Shevchenko
2020-02-21 13:08 ` Petr Mladek
@ 2020-02-21 14:51 ` Uwe Kleine-König
2020-02-21 15:27 ` Andy Shevchenko
1 sibling, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2020-02-21 14:51 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Petr Mladek, Steven Rostedt, Sergey Senozhatsky, linux-kernel,
Rasmus Villemoes
Hello,
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.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> lib/vsprintf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 7c488a1ce318..d5641a217685 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -58,7 +58,7 @@
> * @endp: A pointer to the end of the parsed string will be placed here
> * @base: The number base to use
> *
> - * 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.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-21 14:51 ` Uwe Kleine-König
@ 2020-02-21 15:27 ` Andy Shevchenko
2020-02-21 16:33 ` Uwe Kleine-König
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-21 15:27 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Petr Mladek, Steven Rostedt, Sergey Senozhatsky,
Linux Kernel Mailing List, Rasmus Villemoes
On Fri, Feb 21, 2020 at 4:54 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-21 15:27 ` Andy Shevchenko
@ 2020-02-21 16:33 ` Uwe Kleine-König
2020-02-22 0:28 ` Rasmus Villemoes
0 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2020-02-21 16:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Petr Mladek, Steven Rostedt, Sergey Senozhatsky,
Linux Kernel Mailing List, Rasmus Villemoes
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@pengutronix.de> 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?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-21 16:33 ` Uwe Kleine-König
@ 2020-02-22 0:28 ` Rasmus Villemoes
2020-02-27 13:14 ` Petr Mladek
0 siblings, 1 reply; 10+ messages in thread
From: Rasmus Villemoes @ 2020-02-22 0:28 UTC (permalink / raw)
To: Uwe Kleine-König, Andy Shevchenko
Cc: Andy Shevchenko, Petr Mladek, Steven Rostedt, Sergey Senozhatsky,
Linux Kernel Mailing List, Alexey Dobriyan
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@pengutronix.de> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-22 0:28 ` Rasmus Villemoes
@ 2020-02-27 13:14 ` Petr Mladek
2020-02-27 16:01 ` Rasmus Villemoes
2020-02-28 1:15 ` Sergey Senozhatsky
0 siblings, 2 replies; 10+ messages in thread
From: Petr Mladek @ 2020-02-27 13:14 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Uwe Kleine-König, Andy Shevchenko, Andy Shevchenko,
Steven Rostedt, Sergey Senozhatsky, Linux Kernel Mailing List,
Alexey Dobriyan
On Sat 2020-02-22 01:28:25, Rasmus Villemoes wrote:
> 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@pengutronix.de> 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).
Thanks a lot for the detailed answer. It seems that there is no easy
solution to the problem.
Is still anyone against the original patch updating the comments in
vsprintf.c. Otherwise, I would queue it for 5.7.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-27 13:14 ` Petr Mladek
@ 2020-02-27 16:01 ` Rasmus Villemoes
2020-02-28 14:50 ` Petr Mladek
2020-02-28 1:15 ` Sergey Senozhatsky
1 sibling, 1 reply; 10+ messages in thread
From: Rasmus Villemoes @ 2020-02-27 16:01 UTC (permalink / raw)
To: Petr Mladek, Rasmus Villemoes
Cc: Uwe Kleine-König, Andy Shevchenko, Andy Shevchenko,
Steven Rostedt, Sergey Senozhatsky, Linux Kernel Mailing List,
Alexey Dobriyan
On 27/02/2020 14.14, Petr Mladek wrote:
>
> Is still anyone against the original patch updating the comments in
> vsprintf.c. Otherwise, I would queue it for 5.7.
Please do.
Rasmus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-27 16:01 ` Rasmus Villemoes
@ 2020-02-28 14:50 ` Petr Mladek
0 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2020-02-28 14:50 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Rasmus Villemoes, Uwe Kleine-König, Andy Shevchenko,
Andy Shevchenko, Steven Rostedt, Sergey Senozhatsky,
Linux Kernel Mailing List, Alexey Dobriyan
On Thu 2020-02-27 17:01:15, Rasmus Villemoes wrote:
> On 27/02/2020 14.14, Petr Mladek wrote:
> >
> > Is still anyone against the original patch updating the comments in
> > vsprintf.c. Otherwise, I would queue it for 5.7.
>
> Please do.
The patch has been committed into printk.git, for-5.7 branch.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions
2020-02-27 13:14 ` Petr Mladek
2020-02-27 16:01 ` Rasmus Villemoes
@ 2020-02-28 1:15 ` Sergey Senozhatsky
1 sibling, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 1:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Rasmus Villemoes, Uwe Kleine-König, Andy Shevchenko,
Andy Shevchenko, Steven Rostedt, Sergey Senozhatsky,
Linux Kernel Mailing List, Alexey Dobriyan
On (20/02/27 14:14), Petr Mladek wrote:
>
> Is still anyone against the original patch updating the comments in
> vsprintf.c. Otherwise, I would queue it for 5.7.
>
+1
-ss
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-28 14:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-21 8:57 [PATCH v1] lib/vsprintf: update comment about simple_strto<foo>() functions Andy Shevchenko
2020-02-21 13:08 ` Petr Mladek
2020-02-21 14:51 ` Uwe Kleine-König
2020-02-21 15:27 ` Andy Shevchenko
2020-02-21 16:33 ` Uwe Kleine-König
2020-02-22 0:28 ` Rasmus Villemoes
2020-02-27 13:14 ` Petr Mladek
2020-02-27 16:01 ` Rasmus Villemoes
2020-02-28 14:50 ` Petr Mladek
2020-02-28 1:15 ` Sergey Senozhatsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox