linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Willy Tarreau <w@1wt.eu>
Subject: Re: [PATCH v1] kernel.h: Update comment about simple_strto<foo>() functions
Date: Tue, 27 Feb 2018 13:40:51 +0200	[thread overview]
Message-ID: <1519731651.10722.224.camel@linux.intel.com> (raw)
In-Reply-To: <CANiq72nsPasAxZBkHW0fNcTMj3Vp_QdT3VABs9-mHKe4RHS9KA@mail.gmail.com>

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@linux.intel.com> 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@gmail.com>
> 
> 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.

>  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.

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

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

  reply	other threads:[~2018-02-27 11:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 17:55 [PATCH v1] kernel.h: Update comment about simple_strto<foo>() functions Andy Shevchenko
2018-02-26 22:31 ` Miguel Ojeda
2018-02-27 11:40   ` Andy Shevchenko [this message]
2018-02-27 15:21     ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1519731651.10722.224.camel@linux.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).