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

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Ramsay Jones wrote:

> 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).

[...]
> --- 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