Re: [PATCH 3/7] vcs-svn: fix signedness warnings

[Jonathan Nieder <jrnieder@gmail.com>]

David Barr wrote:

> --- a/vcs-svn/fast_export.c
> +++ b/vcs-svn/fast_export.c
> @@ -259,7 +259,7 @@ static int parse_ls_response(const char *response, uint32_t *mode,
>  	}
>  
>  	/* Mode. */
> -	if (response_end - response < strlen("100644") ||
> +	if (response_end - response < (off_t) strlen("100644") ||

I wish the static analyzer could notice that "response_end - response"
is always nonnegative and stop worrying.  If we want to appease it,
I guess I'd mildly prefer something like

	if (response_end - response < (signed) strlen("100644") ||

which expresses the intent more directly.

[...]
> --- a/vcs-svn/line_buffer.c
> +++ b/vcs-svn/line_buffer.c
> @@ -91,8 +91,7 @@ char *buffer_read_line(struct line_buffer *buf)
>  	return buf->line_buffer;
>  }
>  
> -size_t buffer_read_binary(struct line_buffer *buf,
> -				struct strbuf *sb, size_t size)
> +off_t buffer_read_binary(struct line_buffer *buf, struct strbuf *sb, off_t size)
>  {
>  	return strbuf_fread(sb, size, buf->infile);
>  }

On systems with larger off_t than size_t (think "typical 32-bit PC,
since file offsets tend to be 64 bits"), this silently throws away
bits.  I think the cure is worse than the disease.

[...]
> --- a/vcs-svn/sliding_window.c
> +++ b/vcs-svn/sliding_window.c
> @@ -43,11 +43,11 @@ static int check_offset_overflow(off_t offset, uintmax_t len)
>  	return 0;
>  }
>  
> -int move_window(struct sliding_view *view, off_t off, size_t width)
> +int move_window(struct sliding_view *view, off_t off, off_t width)
>  {

Likewise.  I'd rather the caller know that the window has to fit in an
address space which can be smaller than the maximum file size.

Is this to avoid having two different functions that parse a
variable-length integer, or is there some other reason?

Hope that helps,
Jonathan