Re: [PATCH] lib/kstrtox.c break if overflow is detected

[Levente Kurusa]

Hi,

On Wed, Jan 21, 2015 at 11:26:26AM -0800, Anshul Garg wrote:
> From: Anshul Garg <aksgarg1989@xxxxxxxxx>
> 
> 1. While converting string representation to integer
> break the loop if overflow is detected.
> 2. Clean kstrtoll function
> 
> Signed-off-by: Anshul Garg <aksgarg1989@xxxxxxxxx>
> ---
>  lib/kstrtox.c |   28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index ec8da78..8cbe5ca 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -70,8 +70,10 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
>  		 * it in the max base we support (16)
>  		 */
>  		if (unlikely(res & (~0ull << 60))) {
> -			if (res > div_u64(ULLONG_MAX - val, base))
> +			if (res > div_u64(ULLONG_MAX - val, base)) {
>  				overflow = 1;
> +				break;
> +			}
>  		}
>  		res = res * base + val;
>  		rv++;
> @@ -146,23 +148,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);

Looks correct to me, so:

Reviewed-by: Levente Kurusa <levex@xxxxxxxxx>

But I believe the two hunks are completely unrelated to
eachother and hence should split into a patch series.

Could you please do that and resend? Or if Andrew is OK
with picking it up as is, then it's fine.

Thanks,
    Levente.

Attachment: signature.asc
Description: Digital signature