Re: [PATCH] read_in_full: always report errors
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 26, 2011, 20:53 UTC
- Message-ID
- <7vlixtw5e2.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20110526184839.GA6910@sigill.intra.peff.net>
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");