Re: [PATCH] remove unnecessary test and dead diagnostic

[Jim Meyering <jim@meyering.net>]

Jeff King wrote:
> On Thu, May 26, 2011 at 03:59:14PM +0200, Jim Meyering wrote:
>
>> * sha1_file.c (index_stream): Don't check for size_t < 0.
>> read_in_full does not return an indication of failure.
>
> Are you sure about that?
>
>   $ sed -n '/read_in_full/,/^}/p' wrapper.c
>   ssize_t read_in_full(int fd, void *buf, size_t count)
>   {
>           char *p = buf;
>           ssize_t total = 0;
>
>           while (count > 0) {
>                   ssize_t loaded = xread(fd, p, count);

Argh.  I went in with blinders on, thinking that the caller was
right in using a type of size_t, and then read this "xread" name and
assumed that it would exit upon failure.

Thanks for catching that.
Here's a better patch:

-- >8 --
Subject: [PATCH] use the correct type (ssize_t, not size_t) for read-style function

* sha1_file.c (index_stream): Using an unsigned type,
we would fail to detect a read error and then proceed to
try to write (size_t)-1 bytes.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 sha1_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 5fc877f..8a85217 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2731,11 +2731,11 @@ static int index_stream(unsigned char *sha1, int fd, size_t size,
 	write_or_whine(fast_import.in, fast_import_cmd, len,
 		       "index-stream: feeding fast-import");
 	while (size) {
 		char buf[10240];
 		size_t sz = size < sizeof(buf) ? size : sizeof(buf);
-		size_t actual;
+		ssize_t actual;

 		actual = read_in_full(fd, buf, sz);
 		if (actual < 0)
 			die_errno("index-stream: reading input");
 		if (write_in_full(fast_import.in, buf, actual) != actual)
--
1.7.5.2.660.g9f46c