Re: [PATCH] vcs-svn: Fix some compiler warnings

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 31, 2012, 19:20 UTC
Message-ID: <20120131192053.GC12443@burratino>
In-Reply-To: <4F28378F.6080108@ramsay1.demon.co.uk>

Hi,

Ramsay Jones wrote:

Show 6 quoted lines

> In particular, some versions of gcc complains as follows:
> 
>         CC vcs-svn/sliding_window.o
>     vcs-svn/sliding_window.c: In function `check_overflow':
>     vcs-svn/sliding_window.c:36: warning: comparison is always false \
>         due to limited range of data type

Yuck. Suppressing this warning would presumably also suppress the optimization that notices the comparison is always false.

The -Wtype-limits warning also triggers in some other perfectly reasonable situations: see <http://gcc.gnu.org/PR51712>. I wonder if we should keep a list of unreliable warnings somewhere (e.g., Meta/Make).

[...]

> Note that the "some versions of gcc" which complain includes 3.4.4 and
> 4.1.2, whereas gcc version 4.4.0 compiles the code without complaint.

Thanks for tracking this down. Interesting. -Wtype-limits was split out from the default set of warnings (!) in gcc 4.3 to address <http://gcc.gnu.org/PR12963>, among other bugs (r124875, 2007-05-20).

[...]

Show 11 quoted lines

> --- a/vcs-svn/fast_export.c
> +++ b/vcs-svn/fast_export.c
> @@ -300,7 +300,8 @@ void fast_export_blob_delta(uint32_t mode,
>  				uint32_t len, struct line_buffer *input)
>  {
>  	long postimage_len;
> -	if (len > maximum_signed_value_of_type(off_t))
> +	uintmax_t delta_len = (uintmax_t) len;
> +	if (delta_len > maximum_signed_value_of_type(off_t))
>  		die("enormous delta");
>  	postimage_len = apply_delta((off_t) len, input, old_data, old_mode);

Is there some less ugly way to write the condition "if this value is not representable in this type"?

I guess I could live with something like the following (please don't take the names too seriously):

	static inline off_t off_t_or_die(uintmax_t val, const char *msg_if_bad)
	{
		if (val > maximum_signed_value_of_type(off_t))
			die("%s", msg_if_bad);
		return (off_t) val;
	}

...

		off_t delta_len = off_t_or_die(len, "enormous delta");
		postimage_len = apply_delta(delta_len, input, ...);

What do you think?

Jonathan