LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] lib/kstrtox.c clean kstrtoll function
       [not found] <aksgarg1989@gmai.com>
@ 2015-01-22 13:54 ` Anshul Garg
  2015-01-22 17:25   ` Jeff Epler
  0 siblings, 1 reply; 4+ messages in thread
From: Anshul Garg @ 2015-01-22 13:54 UTC (permalink / raw)
  To: akpm, levex, felipe.contreras, linux-kernel; +Cc: aksgarg1989, anshul.g

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/kstrtox.c clean kstrtoll function
  2015-01-22 13:54 ` [PATCH] lib/kstrtox.c clean kstrtoll function Anshul Garg
@ 2015-01-22 17:25   ` Jeff Epler
  2015-01-23 13:25     ` Anshul Garg
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff Epler @ 2015-01-22 17:25 UTC (permalink / raw)
  To: Anshul Garg; +Cc: akpm, levex, felipe.contreras, linux-kernel, anshul.g

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/kstrtox.c clean kstrtoll function
  2015-01-22 17:25   ` Jeff Epler
@ 2015-01-23 13:25     ` Anshul Garg
  2015-01-24 19:30       ` Jeff Epler
  0 siblings, 1 reply; 4+ messages in thread
From: Anshul Garg @ 2015-01-23 13:25 UTC (permalink / raw)
  To: Jeff Epler; +Cc: akpm, Levente Kurusa, Felipe Contreras, linux-kernel, anshul.g

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/kstrtox.c clean kstrtoll function
  2015-01-23 13:25     ` Anshul Garg
@ 2015-01-24 19:30       ` Jeff Epler
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Epler @ 2015-01-24 19:30 UTC (permalink / raw)
  To: Anshul Garg
  Cc: akpm, Levente Kurusa, Felipe Contreras, linux-kernel, anshul.g


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-01-24 19:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <aksgarg1989@gmai.com>
2015-01-22 13:54 ` [PATCH] lib/kstrtox.c clean kstrtoll function Anshul Garg
2015-01-22 17:25   ` Jeff Epler
2015-01-23 13:25     ` Anshul Garg
2015-01-24 19:30       ` Jeff Epler

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