Re: [PATCH] config.c: fix potential number truncation in git_parse_signed()

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> clang -Wabsolute-value on IA-32 architecture complains that "absolute
> value function 'labs' given an argument of type 'intmax_t' (aka 'long
> long') but has parameter of type 'long' which may cause truncation of
> value". Very unlikely for this code though. Nevertheless, add an
> explicit check for truncation to shut clang up and error out.

Thanks.  It however makes me wonder if it is a better approach to
avoid downcasting intmax_t to long by using imaxabs()?

>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  config.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index d7ce34b..880bd4a 100644
> --- a/config.c
> +++ b/config.c
> @@ -503,6 +503,7 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
>  		intmax_t val;
>  		uintmax_t uval;
>  		uintmax_t factor = 1;
> +		long int lival;
>  
>  		errno = 0;
>  		val = strtoimax(value, &end, 0);
> @@ -512,9 +513,14 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
>  			errno = EINVAL;
>  			return 0;
>  		}
> -		uval = labs(val);
> +		lival = (long int)val;
> +		if (lival != val) {
> +			errno = ERANGE;
> +			return 0;
> +		}
> +		uval = labs(lival);
>  		uval *= factor;
> -		if (uval > max || labs(val) > uval) {
> +		if (uval > max || labs(lival) > uval) {
>  			errno = ERANGE;
>  			return 0;
>  		}