Re: [PATCH] strtoul_ui: actually report error in case of negative input

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Max Kirillov <max@xxxxxxxxxx> writes:

> If s == "-1" and CPU is i386, then none of the checks is triggered, including
> the last "(unsigned int) ul != ul", because ul == 2**32 - 1, which fits into
> "unsigned int".

Thanks for noticing and reporting.

> Fix it by changing the last check to trigger earlier, as soon as it
> becomes bigger than INT_MAX.

What if the value is actually greater than INT_MAX? The function is
returning an unsigned long (64 bits on 64bits architectures), and your
version is restricting it to integers smaller than 2^31, right?

> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -815,7 +815,7 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
>  
>  	errno = 0;
>  	ul = strtoul(s, &p, base);
> -	if (errno || *p || p == s || (unsigned int) ul != ul)
> +	if (errno || *p || p == s || ul > (unsigned long) INT_MAX)

I think you at least want to use LONG_MAX and drop the cast here
(untested, and beware of my advices when given before coffee).
That would restrict to values smaller than 2^63, and I guess no one is
interested in the interval ]2^63, 2^64].

The other option would be to look for a leading '-' before calling
strtoul.

(Actually, this makes me wonder why strtoul happily returns a big
positive when fed with the string "-1", but we can't change it)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]