2007-08-13 14:30:27

by Alexey Dobriyan

[permalink] [raw]
Subject: 2.6.23-rc2-mm2: strtol_check_range patches

Andrew please drop
introduce-strtol_check_range-fix.patch
introduce-strtol_check_range.patch
from -mm.

strtol_check_range() semantics is broken, because caller can't distinguish
-E from valid negative number if he wants to negative integers. Comment
mentions this, but we don't want to such horrible and not well thought
out function to lib/ .

If anything it should be strtonum() with additional trailing '\n' check.

+ * Do not use this to convert numbers that are allowed to be negative.
+ */
+long strtol_check_range(const char *cp, long min, long max, unsigned int base)
+{
+ long ret;
+ char *p = (char *) cp;
+
+ WARN_ON(min < 0);
+ WARN_ON(max < min);
+
+ ret = simple_strtol(p, &p, base);
+
+ if (*p && (*p != '\n'))
+ return -EINVAL;
+ if ((ret < min) || (ret > max))
+ return -EINVAL;
+
+ return ret;
+}


2007-08-13 16:30:29

by Satyam Sharma

[permalink] [raw]
Subject: Re: 2.6.23-rc2-mm2: strtol_check_range patches



On Mon, 13 Aug 2007, Alexey Dobriyan wrote:

> Andrew please drop
> introduce-strtol_check_range-fix.patch
> introduce-strtol_check_range.patch
> from -mm.
>
> strtol_check_range() semantics is broken, because caller can't distinguish
> -E from valid negative number if he wants to negative integers.

As you wrote, the comment does mention that you can't use this to convert
negative integers, and it WARN()'s if the user tries that anyway.

> Comment
> mentions this, but we don't want to such horrible and not well thought
> out function to lib/ .
>
> If anything it should be strtonum() with additional trailing '\n' check.

That's fine, I'll use strtonum() in any case. It turns out the behaviour
that we want is precisely what strtonum() offers, plus that also works for
negative integers, so we might as well just copy it over.

Andrew, I'll diff a new patch implementing strtonum(3) _not_ based on this
one, so you may safely drop this.


Thanks,
Satyam