Re: [PATCH] xsize_t: avoid implementation defined behavior when len < 0

From: Junio C Hamano <gitster@pobox.com>
Date: May 19, 2021, 01:36 UTC
Message-ID: <xmqqy2cbzeqx.fsf@gitster.g>
In-Reply-To: <YKPXVMchtGbwDuue@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 27 quoted lines

> Hi,
>
> This is *not* -rc material; it's just something I noticed and figured
> I would send it before I forget (among other benefits, this helps us
> kick the tires on the release candidate by having patches to work
> with).
>
> Thoughts welcome, as always.
>
> Jonathan
>
>  git-compat-util.h | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index a508dbe5a3..20318a0aac 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -986,11 +986,9 @@ static inline char *xstrdup_or_null(const char *str)
>  
>  static inline size_t xsize_t(off_t len)
>  {
> -	size_t size = (size_t) len;
> -
> -	if (len != (off_t) size)
> +	if (len < 0 || len > SIZE_MAX)
>  		die("Cannot handle files this big");

OK, so negative offset or offset that cannot be represented as size_t are rejected. That is much easier to read than the original ;-)

SIZE_MAX is associated with size_t so it presumably is an unsigned constant; would it again trigger a sign-compare warning?

Show 5 quoted lines

> -	return size;
> +	return (size_t) len;
>  }
>  
>  __attribute__((format (printf, 3, 4)))