Re: [PATCH] read_in_full: always report errors

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> The problem is that most callers are not careful enough to repeatedly
> call read_in_full and find out that there might have been an error in
> the previous result. They see a read shorter than what they asked, and
> assume it was EOF.

I can buy that argument, but then shouldn't we change the "careful"
callers to treat any short-read from read_in_full() as an error?

After this patch, which you convinced me is a good thing to do overall,
they are no longer careful but are merely misguided that they can catch
and tell two kinds of errors apart.

Perhaps like this.  I notice that overall they are good changes, but the
one in pkt-line.c does not look very good.

 combine-diff.c |    5 +----
 csum-file.c    |    2 --
 pkt-line.c     |    6 ++----
 sha1_file.c    |    2 +-
 4 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index be67cfc..176231e 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -845,11 +845,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			result = xmalloc(len + 1);
 
 			done = read_in_full(fd, result, len);
-			if (done < 0)
+			if (done != len)
 				die_errno("read error '%s'", elem->path);
-			else if (done < len)
-				die("early EOF '%s'", elem->path);
-
 			result[len] = 0;
 
 			/* If not a fake symlink, apply filters, e.g. autocrlf */
diff --git a/csum-file.c b/csum-file.c
index fc97d6e..f5ac31f 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -19,8 +19,6 @@ static void flush(struct sha1file *f, void *buf, unsigned int count)
 
 		if (ret < 0)
 			die_errno("%s: sha1 file read error", f->name);
-		if (ret < count)
-			die("%s: sha1 file truncated", f->name);
 		if (memcmp(buf, check_buffer, count))
 			die("sha1 file '%s' validation error", f->name);
 	}
diff --git a/pkt-line.c b/pkt-line.c
index 5a04984..5628801 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -138,10 +138,8 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
 static void safe_read(int fd, void *buffer, unsigned size)
 {
 	ssize_t ret = read_in_full(fd, buffer, size);
-	if (ret < 0)
-		die_errno("read error");
-	else if (ret < size)
-		die("The remote end hung up unexpectedly");
+	if (ret != size)
+		die_errno("The remote end hung up unexpectedly");
 }
 
 static int packet_length(const char *linelen)
diff --git a/sha1_file.c b/sha1_file.c
index 8a85217..d1332c4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2736,7 +2736,7 @@ static int index_stream(unsigned char *sha1, int fd, size_t size,
 		ssize_t actual;
 
 		actual = read_in_full(fd, buf, sz);
-		if (actual < 0)
+		if (actual != sz)
 			die_errno("index-stream: reading input");
 		if (write_in_full(fast_import.in, buf, actual) != actual)
 			die_errno("index-stream: feeding fast-import");