[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LFD.0.999.0708020541060.8258@enigma.security.iitk.ac.in>
Date: Thu, 2 Aug 2007 05:52:02 +0530 (IST)
From: Satyam Sharma <satyam@...radead.org>
To: Alexey Dobriyan <adobriyan@...il.com>
cc: Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH -mm] Introduce strtol_check_range()
On Thu, 2 Aug 2007, Satyam Sharma wrote:
> On Wed, 1 Aug 2007, Alexey Dobriyan wrote:
>
> > On Tue, Jul 31, 2007 at 10:04:10PM +0530, Satyam Sharma wrote:
> > > Callers (especially "store" functions for sysfs or configfs attributes)
> > > that want to convert an input string to a number may often also want to
> > > check for simple input sanity or allowable range. strtol10_check_range()
> > > of netconsole does this, so extract it out into lib/vsprintf.c, make it
> > > generic w.r.t. base, and export it to the rest of the kernel and modules.
> > [...]
> >
> > Please, copy strtonum() from BSD instead. Nobody needs another
> > home-grown converter.
>
> BSD's strtonum(3) is a detestful, horrible shame.
>
> The strtol_check_range() I implemented here does _all_ that strtonum()
> does, plus is generic w.r.t. base, and minus the tasteless "errstr"
> argument.
>
> Tell me, how does that "errstr" ever make sense? We _anyway_ return
> errors (-EINVAL or -ERANGE) if any of those cases show up. And
> _because_ we use negative numbers to return errors, we can't use this
> function to convert negative inputs anyway ... an appropriate error
> message can always be outputted by the caller itself. [ hence the
> two WARN_ON's I added here ]
Glargh, lemme take that back :-p
Wait, it takes in an const char ** argument, and then the whole error
return checking is based on the (const char *) *arg, and not the
return value (-EINVAL, -ERANGE) itself. And it appears, we set the
const char **errstr from inside the function only if the caller didn't
explicitly give us a NULL itself ... hmm, it all does make sense, plus
allows to convert negative inputs as well -- now that the error return
checking in the callsite won't be based on the return value anyway
(but strtonum(3) is still a bit daft, in that it "pretends" to return
error values in ret even though the caller can't use that to test if
an error actually occurred).
[ Thankfully there won't be any BSD lovers around here, so probably I'll
be able to get out of this clean :-) So okay, I'll port it over ... ]
Thanks,
Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists