Re: [PATCH 3/7] vcs-svn: fix signedness warnings
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- May 24, 2012, 14:48 UTC
- Message-ID
- <20120524144847.GC3732@burratino>
- In-Reply-To
- <1337868259-45626-4-git-send-email-davidbarr@google.com>
David Barr wrote:
Show 8 quoted lines
> --- 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.
[...]
Show 12 quoted lines
> --- 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.
[...]
Show 9 quoted lines
> --- 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