Re: [PATCH] config.c: fix potential number truncation in git_parse_signed()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 6, 2016, 19:33 UTC
- Message-ID
- <xmqqa8husgrj.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160702131333.8418-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 5 quoted lines
> 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()?
Show 35 quoted lines
>
> 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;
> }