Re: [PATCH] strtoul_ui: reject negative values

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2015, 16:18 UTC
Message-ID: <xmqqpp1hj9yu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1442500646-15293-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> This patch catches negative values before it's too late, i.e. before
> calling strtoul. We still silently accept very large integers that wrap
> to a valid "unsigned int".

Is the last statement correct? A very large long uint that wrap to uint would not fit in long uint and you would get ERANGE, no?

> So, here's a proper patch (I mean, a band-aid patch, but properly
> send ;-) ).

Yup.

> It should be merged before Kartik's series (or inserted at the start
> of the series) so that we get the fix before the test breakage.

Which one of his series?

Show 18 quoted lines

>
>  git-compat-util.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f649e81..1df82fa 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -814,6 +814,9 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
>  	char *p;
>  
>  	errno = 0;
> +	/* negative values would be accepted by strtoul */
> +	if (strchr(s, '-'))
> +		return -1;
>  	ul = strtoul(s, &p, base);
>  	if (errno || *p || p == s || (unsigned int) ul != ul)
>  		return -1;