[PATCH] lib/kstrtox.c clean kstrtoll function

[PATCH] lib/kstrtox.c clean kstrtoll function

[Anshul Garg]

From: Anshul Garg <aksgarg1989@gmail.com>

Instead of having same code for negative and postive
integer, use sign variable for integer parsing.

Signed-off-by: Anshul Garg <aksgarg1989@gmail.com>
---
 lib/kstrtox.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index ec8da78..744cbb0 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -146,23 +146,19 @@ EXPORT_SYMBOL(kstrtoull);
 int kstrtoll(const char *s, unsigned int base, long long *res)
 {
 	unsigned long long tmp;
-	int rv;
+	int rv, sign = 1;
 
 	if (s[0] == '-') {
-		rv = _kstrtoull(s + 1, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)(-tmp) >= 0)
-			return -ERANGE;
-		*res = -tmp;
-	} else {
-		rv = kstrtoull(s, base, &tmp);
-		if (rv < 0)
-			return rv;
-		if ((long long)tmp < 0)
-			return -ERANGE;
-		*res = tmp;
+		sign = -1;
+		s++;
 	}
+
+	rv = kstrtoull(s, base, &tmp);
+	if (rv < 0)
+		return rv;
+	if ((long long)tmp < 0)
+		return -ERANGE;
+	*res = sign * tmp;
 	return 0;
 }
 EXPORT_SYMBOL(kstrtoll);
-- 
1.7.9.5


---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com

Re: [PATCH] lib/kstrtox.c clean kstrtoll function

[Jeff Epler]

On Thu, Jan 22, 2015 at 05:54:10AM -0800, Anshul Garg wrote:
> -		if ((long long)(-tmp) >= 0)
> -			return -ERANGE;
> -		*res = -tmp;
...
> +	if ((long long)tmp < 0)
> +		return -ERANGE;
> +	*res = sign * tmp;

I don't believe overflow handling is correct anymore with this patch.
Did you try with the input as the most negative possible unsigned long?

Jeff

Re: [PATCH] lib/kstrtox.c clean kstrtoll function

[Anshul Garg]

Dear Mr. Jeff,

Thanks for the comments.

Yes i think overflow check logic is wrong.

So i think we can change the overflow logic -

>From --

if ((long long)tmp < 0)
+ return -ERANGE;

to -

if (((long long)tmp < LLONG_MIN) || ((long long)tmp > LLONG_MAX) )
+ return -ERANGE;

Please give your views on this change..
If this change seems correct i will update overflow logic
in my sent patch.


Hope to hear from you soon.

Thanks
Anshul Garg


On Thu, Jan 22, 2015 at 10:55 PM, Jeff Epler <jepler@unpythonic.net> wrote:
> On Thu, Jan 22, 2015 at 05:54:10AM -0800, Anshul Garg wrote:
>> -             if ((long long)(-tmp) >= 0)
>> -                     return -ERANGE;
>> -             *res = -tmp;
> ...
>> +     if ((long long)tmp < 0)
>> +             return -ERANGE;
>> +     *res = sign * tmp;
>
> I don't believe overflow handling is correct anymore with this patch.
> Did you try with the input as the most negative possible unsigned long?
>
> Jeff

Re: [PATCH] lib/kstrtox.c clean kstrtoll function

[Jeff Epler]

On Fri, Jan 23, 2015 at 06:55:36PM +0530, Anshul Garg wrote:
> if (((long long)tmp < LLONG_MIN) || ((long long)tmp > LLONG_MAX) )
> + return -ERANGE;

This proposed code is still wrong (-ERANGE can never be returned by this
statement).  It may be best to leave the code alone, rather than propose
more untested alternatives.

Jeff