[PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[Andy Shevchenko]

There were discussions in the past about use cases for
simple_strto<foo>() functions and, in some rare cases,
they have a benefit over kstrto<foo>() ones.

Update a comment to reduce confusion about special use cases.

Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- update comment based on Geert's input
 include/linux/kernel.h | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 0c9bc231107f..63663c44933d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -332,8 +332,7 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
  * @res: Where to write the result of the conversion on success.
  *
  * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
+ * Used as a replacement for the simple_strtoull. Return code must be checked.
 */
 static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
 {
@@ -361,8 +360,7 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
  * @res: Where to write the result of the conversion on success.
  *
  * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
- * Used as a replacement for the obsolete simple_strtoull. Return code must
- * be checked.
+ * Used as a replacement for the simple_strtoull. Return code must be checked.
  */
 static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
 {
@@ -438,7 +436,16 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
 	return kstrtoint_from_user(s, count, base, res);
 }
 
-/* Obsolete, do not use.  Use kstrto<foo> instead */
+/*
+ * Use kstrto<foo> instead.
+ *
+ * NOTE: The simple_strto<foo> does not check for overflow and,
+ *	 depending on the input, may give interesting results.
+ *
+ * Use these functions if and only if you cannot use kstrto<foo>, because
+ * the number is not immediately followed by a NUL-character in the buffer.
+ * Keep in mind above caveat.
+ */
 
 extern unsigned long simple_strtoul(const char *,char **,unsigned int);
 extern long simple_strtol(const char *,char **,unsigned int);
-- 
2.20.1

[PATCH v3 2/2] auxdisplay: charlcd: Deduplicate simple_strtoul()

[Andy Shevchenko]

Like in the commit
  8b2303de399f ("serial: core: Fix handling of options after MMIO address")
we may use simple_strtoul() which in comparison to kstrtoul() can do conversion
in-place without additional and unnecessary code to be written.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- no change since v2
 drivers/auxdisplay/charlcd.c | 34 +++++++---------------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 92745efefb54..3858dc7a4154 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -287,31 +287,6 @@ static int charlcd_init_display(struct charlcd *lcd)
 	return 0;
 }
 
-/*
- * Parses an unsigned integer from a string, until a non-digit character
- * is found. The empty string is not accepted. No overflow checks are done.
- *
- * Returns whether the parsing was successful. Only in that case
- * the output parameters are written to.
- *
- * TODO: If the kernel adds an inplace version of kstrtoul(), this function
- * could be easily replaced by that.
- */
-static bool parse_n(const char *s, unsigned long *res, const char **next_s)
-{
-	if (!isdigit(*s))
-		return false;
-
-	*res = 0;
-	while (isdigit(*s)) {
-		*res = *res * 10 + (*s - '0');
-		++s;
-	}
-
-	*next_s = s;
-	return true;
-}
-
 /*
  * Parses a movement command of the form "(.*);", where the group can be
  * any number of subcommands of the form "(x|y)[0-9]+".
@@ -336,6 +311,7 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
 {
 	unsigned long new_x = *x;
 	unsigned long new_y = *y;
+	char *p;
 
 	for (;;) {
 		if (!*s)
@@ -345,11 +321,15 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
 			break;
 
 		if (*s == 'x') {
-			if (!parse_n(s + 1, &new_x, &s))
+			new_x = simple_strtoul(s + 1, &p, 10);
+			if (p == s + 1)
 				return false;
+			s = p;
 		} else if (*s == 'y') {
-			if (!parse_n(s + 1, &new_y, &s))
+			new_y = simple_strtoul(s + 1, &p, 10);
+			if (p == s + 1)
 				return false;
+			s = p;
 		} else {
 			return false;
 		}
-- 
2.20.1

Re: [PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[Petr Mladek]

On Thu 2019-07-04 14:55:31, Andy Shevchenko wrote:
> There were discussions in the past about use cases for
> simple_strto<foo>() functions and, in some rare cases,
> they have a benefit over kstrto<foo>() ones.
> 
> Update a comment to reduce confusion about special use cases.
> 
> Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - update comment based on Geert's input
>  include/linux/kernel.h | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 0c9bc231107f..63663c44933d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -332,8 +332,7 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
>   * @res: Where to write the result of the conversion on success.
>   *
>   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> - * Used as a replacement for the obsolete simple_strtoull. Return code must
> - * be checked.
> + * Used as a replacement for the simple_strtoull. Return code must be checked.
>  */
>  static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
>  {
> @@ -361,8 +360,7 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
>   * @res: Where to write the result of the conversion on success.
>   *
>   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> - * Used as a replacement for the obsolete simple_strtoull. Return code must
> - * be checked.
> + * Used as a replacement for the simple_strtoull. Return code must be checked.
>   */
>  static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
>  {
> @@ -438,7 +436,16 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
>  	return kstrtoint_from_user(s, count, base, res);
>  }
>  
> -/* Obsolete, do not use.  Use kstrto<foo> instead */
> +/*
> + * Use kstrto<foo> instead.
> + *
> + * NOTE: The simple_strto<foo> does not check for overflow and,
> + *	 depending on the input, may give interesting results.

I am a bit confused whether the interesting results are caused
by the buffer overflow or if there is another reason.

If it is because of the overflow, I would remove the 2nd line. I guess
that anyone knows what a buffer overflow might cause.

Otherwise the patch looks fine to me.

Best Regards,
Petr

Re: [PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[Geert Uytterhoeven]

Hi Petr,

On Mon, Jul 8, 2019 at 3:02 PM Petr Mladek <pmladek@suse.com> wrote:
> On Thu 2019-07-04 14:55:31, Andy Shevchenko wrote:
> > There were discussions in the past about use cases for
> > simple_strto<foo>() functions and, in some rare cases,
> > they have a benefit over kstrto<foo>() ones.
> >
> > Update a comment to reduce confusion about special use cases.
> >
> > Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > - update comment based on Geert's input
> >  include/linux/kernel.h | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 0c9bc231107f..63663c44933d 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -332,8 +332,7 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
> >   * @res: Where to write the result of the conversion on success.
> >   *
> >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > - * be checked.
> > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> >  */
> >  static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
> >  {
> > @@ -361,8 +360,7 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
> >   * @res: Where to write the result of the conversion on success.
> >   *
> >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > - * be checked.
> > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> >   */
> >  static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
> >  {
> > @@ -438,7 +436,16 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
> >       return kstrtoint_from_user(s, count, base, res);
> >  }
> >
> > -/* Obsolete, do not use.  Use kstrto<foo> instead */
> > +/*
> > + * Use kstrto<foo> instead.
> > + *
> > + * NOTE: The simple_strto<foo> does not check for overflow and,
> > + *    depending on the input, may give interesting results.
>
> I am a bit confused whether the interesting results are caused
> by the buffer overflow or if there is another reason.

Which buffer overflow?
> If it is because of the overflow, I would remove the 2nd line. I guess
> that anyone knows what a buffer overflow might cause.

AFAIK, the overflow is a numerical overflow.

The "interesting result" is that the function keeps parsing until it finds
a character that doesn't fit in the range of expected characters, according
to the specified numerical base, but further ignoring character class.
But that's really what you want, when you want to parse things like
10x50 or 10:50.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/2] auxdisplay: charlcd: Deduplicate simple_strtoul()

[Petr Mladek]

On Thu 2019-07-04 14:55:32, Andy Shevchenko wrote:
> Like in the commit
>   8b2303de399f ("serial: core: Fix handling of options after MMIO address")
> we may use simple_strtoul() which in comparison to kstrtoul() can do conversion
> in-place without additional and unnecessary code to be written.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - no change since v2
>  drivers/auxdisplay/charlcd.c | 34 +++++++---------------------------
>  1 file changed, 7 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
> index 92745efefb54..3858dc7a4154 100644
> --- a/drivers/auxdisplay/charlcd.c
> +++ b/drivers/auxdisplay/charlcd.c
> @@ -287,31 +287,6 @@ static int charlcd_init_display(struct charlcd *lcd)
>  	return 0;
>  }
>  
> -/*
> - * Parses an unsigned integer from a string, until a non-digit character
> - * is found. The empty string is not accepted. No overflow checks are done.
> - *
> - * Returns whether the parsing was successful. Only in that case
> - * the output parameters are written to.
> - *
> - * TODO: If the kernel adds an inplace version of kstrtoul(), this function
> - * could be easily replaced by that.
> - */
> -static bool parse_n(const char *s, unsigned long *res, const char **next_s)
> -{
> -	if (!isdigit(*s))
> -		return false;
> -
> -	*res = 0;
> -	while (isdigit(*s)) {
> -		*res = *res * 10 + (*s - '0');
> -		++s;
> -	}
> -
> -	*next_s = s;
> -	return true;
> -}
> -
>  /*
>   * Parses a movement command of the form "(.*);", where the group can be
>   * any number of subcommands of the form "(x|y)[0-9]+".
> @@ -336,6 +311,7 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
>  {
>  	unsigned long new_x = *x;
>  	unsigned long new_y = *y;
> +	char *p;
>  
>  	for (;;) {
>  		if (!*s)
> @@ -345,11 +321,15 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
>  			break;
>  
>  		if (*s == 'x') {
> -			if (!parse_n(s + 1, &new_x, &s))
> +			new_x = simple_strtoul(s + 1, &p, 10);

simple_strtoul() tries to detect the base even when it has been
explicitely specified. I am afraid that it might cause some
regressions.

For example, the following input is strange but it is valid:

    x0x10;  new code would return (16, <orig_y>) instead of (10, <orig_y>)
    x010;   new code would return (8, <orig_y>) instead of (10, <orig_y>)

Best Regards,
Petr

Re: [PATCH v3 2/2] auxdisplay: charlcd: Deduplicate simple_strtoul()

[Andy Shevchenko]

On Mon, Jul 08, 2019 at 03:16:52PM +0200, Petr Mladek wrote:
> On Thu 2019-07-04 14:55:32, Andy Shevchenko wrote:
> > Like in the commit
> >   8b2303de399f ("serial: core: Fix handling of options after MMIO address")
> > we may use simple_strtoul() which in comparison to kstrtoul() can do conversion
> > in-place without additional and unnecessary code to be written.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > - no change since v2
> >  drivers/auxdisplay/charlcd.c | 34 +++++++---------------------------
> >  1 file changed, 7 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
> > index 92745efefb54..3858dc7a4154 100644
> > --- a/drivers/auxdisplay/charlcd.c
> > +++ b/drivers/auxdisplay/charlcd.c
> > @@ -287,31 +287,6 @@ static int charlcd_init_display(struct charlcd *lcd)
> >  	return 0;
> >  }
> >  
> > -/*
> > - * Parses an unsigned integer from a string, until a non-digit character
> > - * is found. The empty string is not accepted. No overflow checks are done.
> > - *
> > - * Returns whether the parsing was successful. Only in that case
> > - * the output parameters are written to.
> > - *
> > - * TODO: If the kernel adds an inplace version of kstrtoul(), this function
> > - * could be easily replaced by that.
> > - */
> > -static bool parse_n(const char *s, unsigned long *res, const char **next_s)
> > -{
> > -	if (!isdigit(*s))
> > -		return false;
> > -
> > -	*res = 0;
> > -	while (isdigit(*s)) {
> > -		*res = *res * 10 + (*s - '0');
> > -		++s;
> > -	}
> > -
> > -	*next_s = s;
> > -	return true;
> > -}
> > -
> >  /*
> >   * Parses a movement command of the form "(.*);", where the group can be
> >   * any number of subcommands of the form "(x|y)[0-9]+".
> > @@ -336,6 +311,7 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
> >  {
> >  	unsigned long new_x = *x;
> >  	unsigned long new_y = *y;
> > +	char *p;
> >  
> >  	for (;;) {
> >  		if (!*s)
> > @@ -345,11 +321,15 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
> >  			break;
> >  
> >  		if (*s == 'x') {
> > -			if (!parse_n(s + 1, &new_x, &s))
> > +			new_x = simple_strtoul(s + 1, &p, 10);
> 
> simple_strtoul() tries to detect the base even when it has been
> explicitely specified. 

I can't see it from the code. Can you point out to this drastic bug that has to
be fixed?

> I am afraid that it might cause some
> regressions.
> 
> For example, the following input is strange but it is valid:
> 
>     x0x10;  new code would return (16, <orig_y>) instead of (10, <orig_y>)
>     x010;   new code would return (8, <orig_y>) instead of (10, <orig_y>)
> 
> Best Regards,
> Petr

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v3 2/2] auxdisplay: charlcd: Deduplicate simple_strtoul()

[David Laight]

> > simple_strtoul() tries to detect the base even when it has been
> > explicitely specified.
> 
> I can't see it from the code. Can you point out to this drastic bug that has to
> be fixed?
> 
> > I am afraid that it might cause some
> > regressions.
> >
> > For example, the following input is strange but it is valid:
> >
> >     x0x10;  new code would return (16, <orig_y>) instead of (10, <orig_y>)
> >     x010;   new code would return (8, <orig_y>) instead of (10, <orig_y>)

While having simple_stroul("0x10", 10) use base 16 is possibly not unreasonable,
using base 8 for simple_strtoul("010", 10) is just plain wrong.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[Petr Mladek]

On Mon 2019-07-08 15:13:50, Geert Uytterhoeven wrote:
> Hi Petr,
> 
> On Mon, Jul 8, 2019 at 3:02 PM Petr Mladek <pmladek@suse.com> wrote:
> > On Thu 2019-07-04 14:55:31, Andy Shevchenko wrote:
> > > There were discussions in the past about use cases for
> > > simple_strto<foo>() functions and, in some rare cases,
> > > they have a benefit over kstrto<foo>() ones.
> > >
> > > Update a comment to reduce confusion about special use cases.
> > >
> > > Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > - update comment based on Geert's input
> > >  include/linux/kernel.h | 17 ++++++++++++-----
> > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > > index 0c9bc231107f..63663c44933d 100644
> > > --- a/include/linux/kernel.h
> > > +++ b/include/linux/kernel.h
> > > @@ -332,8 +332,7 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
> > >   * @res: Where to write the result of the conversion on success.
> > >   *
> > >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > > - * be checked.
> > > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> > >  */
> > >  static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
> > >  {
> > > @@ -361,8 +360,7 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
> > >   * @res: Where to write the result of the conversion on success.
> > >   *
> > >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > > - * be checked.
> > > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> > >   */
> > >  static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
> > >  {
> > > @@ -438,7 +436,16 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
> > >       return kstrtoint_from_user(s, count, base, res);
> > >  }
> > >
> > > -/* Obsolete, do not use.  Use kstrto<foo> instead */
> > > +/*
> > > + * Use kstrto<foo> instead.
> > > + *
> > > + * NOTE: The simple_strto<foo> does not check for overflow and,
> > > + *    depending on the input, may give interesting results.
> >
> > I am a bit confused whether the interesting results are caused
> > by the buffer overflow or if there is another reason.
> 
> Which buffer overflow?
> > If it is because of the overflow, I would remove the 2nd line. I guess
> > that anyone knows what a buffer overflow might cause.
> 
> AFAIK, the overflow is a numerical overflow.
> 
> The "interesting result" is that the function keeps parsing until it finds
> a character that doesn't fit in the range of expected characters, according
> to the specified numerical base, but further ignoring character class.
> But that's really what you want, when you want to parse things like
> 10x50 or 10:50.

Thanks for the detailed explanation. It means that the new text is
still confusing. Please, make it more explicit, e.g.

NOTE: simple_strto<foo> does not check for the range overflow.

      The conversion ends on the first non-number character. It is
      needed only for parsing strings like 10;50; or 10:50 without
      the need to modify the original string.

      Be aware that the number base is being detected. Therefore, for
      example, "0x1a" returns 26 (base 16) and "019" returns 1 (base 8).

Best Regards,
Petr

Re: [PATCH v3 2/2] auxdisplay: charlcd: Deduplicate simple_strtoul()

[Petr Mladek]

On Mon 2019-07-08 16:35:34, Andy Shevchenko wrote:
> On Mon, Jul 08, 2019 at 03:16:52PM +0200, Petr Mladek wrote:
> > On Thu 2019-07-04 14:55:32, Andy Shevchenko wrote:
> > > Like in the commit
> > >   8b2303de399f ("serial: core: Fix handling of options after MMIO address")
> > > we may use simple_strtoul() which in comparison to kstrtoul() can do conversion
> > > in-place without additional and unnecessary code to be written.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > - no change since v2
> > >  drivers/auxdisplay/charlcd.c | 34 +++++++---------------------------
> > >  1 file changed, 7 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
> > > index 92745efefb54..3858dc7a4154 100644
> > > --- a/drivers/auxdisplay/charlcd.c
> > > +++ b/drivers/auxdisplay/charlcd.c
> > > @@ -287,31 +287,6 @@ static int charlcd_init_display(struct charlcd *lcd)
> > >  	return 0;
> > >  }
> > >  
> > > -/*
> > > - * Parses an unsigned integer from a string, until a non-digit character
> > > - * is found. The empty string is not accepted. No overflow checks are done.
> > > - *
> > > - * Returns whether the parsing was successful. Only in that case
> > > - * the output parameters are written to.
> > > - *
> > > - * TODO: If the kernel adds an inplace version of kstrtoul(), this function
> > > - * could be easily replaced by that.
> > > - */
> > > -static bool parse_n(const char *s, unsigned long *res, const char **next_s)
> > > -{
> > > -	if (!isdigit(*s))
> > > -		return false;
> > > -
> > > -	*res = 0;
> > > -	while (isdigit(*s)) {
> > > -		*res = *res * 10 + (*s - '0');
> > > -		++s;
> > > -	}
> > > -
> > > -	*next_s = s;
> > > -	return true;
> > > -}
> > > -
> > >  /*
> > >   * Parses a movement command of the form "(.*);", where the group can be
> > >   * any number of subcommands of the form "(x|y)[0-9]+".
> > > @@ -336,6 +311,7 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
> > >  {
> > >  	unsigned long new_x = *x;
> > >  	unsigned long new_y = *y;
> > > +	char *p;
> > >  
> > >  	for (;;) {
> > >  		if (!*s)
> > > @@ -345,11 +321,15 @@ static bool parse_xy(const char *s, unsigned long *x, unsigned long *y)
> > >  			break;
> > >  
> > >  		if (*s == 'x') {
> > > -			if (!parse_n(s + 1, &new_x, &s))
> > > +			new_x = simple_strtoul(s + 1, &p, 10);
> > 
> > simple_strtoul() tries to detect the base even when it has been
> > explicitely specified. 
> 
> I can't see it from the code. Can you point out to this drastic bug that has to
> be fixed?

Grr, the base is detected only when it was not defined.

I am sorry for the noise. I probably have not woken up fully after
the weekend yet.

The patch looks fine then. Feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr