[PATCH v2] fetch-pack: try harder to read an ERR packet

[Christian Couder <christian.couder@gmail.com>]

From: SZEDER Gábor <szeder.dev@gmail.com>

When the server has hung up after sending an ERR packet to the
client, the client might still be writing, for example a "done"
line. Therefore the client might get a write error before reading
the ERR packet.

When fetching, this could result in the client displaying a
"Broken pipe" error, instead of the more useful error sent by
the server in the ERR packet.

Instead of using write_in_full() which immediately returns an
error when the server has hung up, let's use a new
write_in_full_read_err() function which will read all the
packets left before returning an error.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 cache.h      | 10 +++++++++-
 fetch-pack.c | 13 +++++++------
 wrapper.c    |  7 +++++--
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 0f0485ecfe..efeee637b8 100644
--- a/cache.h
+++ b/cache.h
@@ -1808,10 +1808,18 @@ int copy_file_with_time(const char *dst, const char *src, int mode);
 void write_or_die(int fd, const void *buf, size_t count);
 void fsync_or_die(int fd, const char *);
 
+struct packet_reader;
+
 ssize_t read_in_full(int fd, void *buf, size_t count);
-ssize_t write_in_full(int fd, const void *buf, size_t count);
+ssize_t write_in_full_read_err(int fd, const void *buf, size_t count,
+			       struct packet_reader *reader);
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
 
+static inline ssize_t write_in_full(int fd, const void *buf, size_t count)
+{
+	return write_in_full_read_err(fd, buf, count, NULL);
+}
+
 static inline ssize_t write_str_in_full(int fd, const char *str)
 {
 	return write_in_full(fd, str, strlen(str));
diff --git a/fetch-pack.c b/fetch-pack.c
index 0b07b3ee73..febdf54bf4 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -185,13 +185,14 @@ static enum ack_type get_ack(struct packet_reader *reader,
 }
 
 static void send_request(struct fetch_pack_args *args,
-			 int fd, struct strbuf *buf)
+			 int fd, struct strbuf *buf,
+			 struct packet_reader *reader)
 {
 	if (args->stateless_rpc) {
 		send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
 		packet_flush(fd);
 	} else {
-		if (write_in_full(fd, buf->buf, buf->len) < 0)
+		if (write_in_full_read_err(fd, buf->buf, buf->len, reader) < 0)
 			die_errno(_("unable to write to remote"));
 	}
 }
@@ -349,7 +350,7 @@ static int find_common(struct fetch_negotiator *negotiator,
 		const char *arg;
 		struct object_id oid;
 
-		send_request(args, fd[1], &req_buf);
+		send_request(args, fd[1], &req_buf, &reader);
 		while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
 			if (skip_prefix(reader.line, "shallow ", &arg)) {
 				if (get_oid_hex(arg, &oid))
@@ -372,7 +373,7 @@ static int find_common(struct fetch_negotiator *negotiator,
 			die(_("expected shallow/unshallow, got %s"), reader.line);
 		}
 	} else if (!args->stateless_rpc)
-		send_request(args, fd[1], &req_buf);
+		send_request(args, fd[1], &req_buf, &reader);
 
 	if (!args->stateless_rpc) {
 		/* If we aren't using the stateless-rpc interface
@@ -395,7 +396,7 @@ static int find_common(struct fetch_negotiator *negotiator,
 			int ack;
 
 			packet_buf_flush(&req_buf);
-			send_request(args, fd[1], &req_buf);
+			send_request(args, fd[1], &req_buf, &reader);
 			strbuf_setlen(&req_buf, state_len);
 			flushes++;
 			flush_at = next_flush(args->stateless_rpc, count);
@@ -470,7 +471,7 @@ static int find_common(struct fetch_negotiator *negotiator,
 	trace2_region_leave("fetch-pack", "negotiation_v0_v1", the_repository);
 	if (!got_ready || !no_done) {
 		packet_buf_write(&req_buf, "done\n");
-		send_request(args, fd[1], &req_buf);
+		send_request(args, fd[1], &req_buf, &reader);
 	}
 	print_verbose(args, _("done"));
 	if (retval != 0) {
diff --git a/wrapper.c b/wrapper.c
index 3a1c0e0526..2da7a15382 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "config.h"
+#include "pkt-line.h"
 
 static int memory_limit_check(size_t size, int gentle)
 {
@@ -291,7 +292,8 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
 	return total;
 }
 
-ssize_t write_in_full(int fd, const void *buf, size_t count)
+ssize_t write_in_full_read_err(int fd, const void *buf, size_t count,
+			       struct packet_reader *reader)
 {
 	const char *p = buf;
 	ssize_t total = 0;
@@ -300,7 +302,8 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 		ssize_t written = xwrite(fd, p, count);
 		if (written < 0)
 			return -1;
-		if (!written) {
+		if (!written &&
+		    (!reader || packet_reader_read(reader) == PACKET_READ_EOF)) {
 			errno = ENOSPC;
 			return -1;
 		}
-- 
2.26.2.267.g8b5eb5fb58.dirty