Re: [PATCH 06/16] vcs-svn: skeleton of an svn delta parser

[Jonathan Nieder <jrnieder@gmail.com>]

(culling cc list)
Hi,

David Barr wrote:

> --- a/vcs-svn/line_buffer.c
> +++ b/vcs-svn/line_buffer.c
> @@ -98,10 +98,10 @@ char *buffer_read_string(struct line_buffer *buf, uint32_t len)
>  	return ferror(buf->infile) ? NULL : buf->blob_buffer.buf;
>  }
>  
> -void buffer_read_binary(struct line_buffer *buf,
> -				struct strbuf *sb, uint32_t size)
> +off_t buffer_read_binary(struct line_buffer *buf,
> +				struct strbuf *sb, off_t size)
>  {
> -	strbuf_fread(sb, size, buf->infile);
> +	return strbuf_fread(sb, size, buf->infile);
>  }

Apparently this change is from in an evil merge.  Yikes.

Anyway, I think the original patch was something like the following.
Would you mind if the parameter and return value go back to being of
type size_t (to avoid a possibly problematic conversion when passing
values to and from strbuf_fread)?

-- 8< --
Date: Sun, 2 Jan 2011 21:37:36 -0600
Subject: vcs-svn: make buffer_read_binary API more convenient

buffer_read_binary is a thin wrapper around fread, but its signature
is wrong:

 - fread can fill an arbitrary in-memory buffer.  buffer_read_binary
   is limited to buffers whose size is representable by a 32-bit
   integer.
 - The result from fread is the number of bytes actually read.
   buffer_read_binary only reports the number of bytes read by
   incrementing sb->len by that amount and returns void.

Fix both: let buffer_read_binary accept a size_t instead of uint32_t
for the number of bytes to try to read and as a convenience return the
number of bytes read.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 vcs-svn/line_buffer.c |    6 +++---
 vcs-svn/line_buffer.h |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index c390387..01fcb84 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -91,10 +91,10 @@ char *buffer_read_line(struct line_buffer *buf)
 	return buf->line_buffer;
 }
 
-void buffer_read_binary(struct line_buffer *buf,
-				struct strbuf *sb, uint32_t size)
+size_t buffer_read_binary(struct line_buffer *buf,
+				struct strbuf *sb, size_t size)
 {
-	strbuf_fread(sb, size, buf->infile);
+	return strbuf_fread(sb, size, buf->infile);
 }
 
 off_t buffer_copy_bytes(struct line_buffer *buf, off_t nbytes)
diff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h
index d0b22dd..8901f21 100644
--- a/vcs-svn/line_buffer.h
+++ b/vcs-svn/line_buffer.h
@@ -23,7 +23,7 @@ long buffer_tmpfile_prepare_to_read(struct line_buffer *buf);
 int buffer_ferror(struct line_buffer *buf);
 char *buffer_read_line(struct line_buffer *buf);
 int buffer_read_char(struct line_buffer *buf);
-void buffer_read_binary(struct line_buffer *buf, struct strbuf *sb, uint32_t len);
+size_t buffer_read_binary(struct line_buffer *buf, struct strbuf *sb, size_t len);
 /* Returns number of bytes read (not necessarily written). */
 off_t buffer_copy_bytes(struct line_buffer *buf, off_t len);
 off_t buffer_skip_bytes(struct line_buffer *buf, off_t len);
-- 
1.7.4.2.660.g270d4b.dirty