Re: [PATCH -mm] Introduce strtol_check_range()

[Satyam Sharma]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/