[PATCHv2 03/10] pkt-line: clean up "gentle" reading function

[Jeff King <peff@peff.net>]

Originally we had a single function for reading packetized
data: packet_read_line. Commit 46284dd grew a more "gentle"
form that would return an error instead of dying upon
reading a truncated input stream. However:

  1. The two functions were called "packet_read" and
     "packet_read_line", with no indication that the only
     difference is in the error handling.

  2. There was no documentation about which error conditions
     were handled in the gentle form, and which still caused
     a death.

  3. The internal variable to trigger the gentle mode was
     called "return_line_fail", which was not very
     expressive.

This patch converts packet_line to packet_read_line_gently
to more clearly indicate its relationship to
packet_read_line, and renames the internal variable to
"gently". This is also not incredibly expressive, but it is
at least a convention within the git code. And finally, we
document the exact behavior for the gentle and non-gentle
modes.

While we are cleaning up the names, we can drop the
"return_line_fail" checks in packet_read_internal entirely.
They look like this:

  ret = safe_read(..., return_line_fail);
  if (return_line_fail && ret < 0)
	  ...

The check for return_line_fail is a no-op; safe_read will
only ever return an error value if we passed it
return_line_fail in the first place.

Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this one is a matter of taste, but I think the result is much
better. Certainly the documentation bits are hard to argue with. :)

 connect.c  |  2 +-
 pkt-line.c | 16 ++++++++--------
 pkt-line.h | 21 ++++++++++++++++++++-
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/connect.c b/connect.c
index 49e56ba..7e0920d 100644
--- a/connect.c
+++ b/connect.c
@@ -76,7 +76,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
 		char *name;
 		int len, name_len;
 
-		len = packet_read(in, buffer, sizeof(buffer));
+		len = packet_read_line_gently(in, buffer, sizeof(buffer));
 		if (len < 0)
 			die_initial_contact(got_at_least_one_head);
 
diff --git a/pkt-line.c b/pkt-line.c
index 699c2dd..62479d3 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -103,13 +103,13 @@ static int safe_read(int fd, void *buffer, unsigned size, int return_line_fail)
 	strbuf_add(buf, buffer, n);
 }
 
-static int safe_read(int fd, void *buffer, unsigned size, int return_line_fail)
+static int safe_read(int fd, void *buffer, unsigned size, int gently)
 {
 	ssize_t ret = read_in_full(fd, buffer, size);
 	if (ret < 0)
 		die_errno("read error");
 	else if (ret < size) {
-		if (return_line_fail)
+		if (gently)
 			return -1;
 
 		die("The remote end hung up unexpectedly");
@@ -143,13 +143,13 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int return_
 	return len;
 }
 
-static int packet_read_internal(int fd, char *buffer, unsigned size, int return_line_fail)
+static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
 {
 	int len, ret;
 	char linelen[4];
 
-	ret = safe_read(fd, linelen, 4, return_line_fail);
-	if (return_line_fail && ret < 0)
+	ret = safe_read(fd, linelen, 4, gently);
+	if (ret < 0)
 		return ret;
 	len = packet_length(linelen);
 	if (len < 0)
@@ -161,15 +161,15 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int return_
 	len -= 4;
 	if (len >= size)
 		die("protocol error: bad line length %d", len);
-	ret = safe_read(fd, buffer, len, return_line_fail);
-	if (return_line_fail && ret < 0)
+	ret = safe_read(fd, buffer, len, gently);
+	if (ret < 0)
 		return ret;
 	buffer[len] = 0;
 	packet_trace(buffer, len, 0);
 	return len;
 }
 
-int packet_read(int fd, char *buffer, unsigned size)
+int packet_read_line_gently(int fd, char *buffer, unsigned size)
 {
 	return packet_read_internal(fd, buffer, size, 1);
 }
diff --git a/pkt-line.h b/pkt-line.h
index 7a67e9c..31bd069 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -24,8 +24,27 @@ int packet_read_line(int fd, char *buffer, unsigned size);
 void packet_buf_flush(struct strbuf *buf);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
 
+/*
+ * Read a packetized line from the descriptor into the buffer. We will die
+ * under any of the following conditions:
+ *
+ *   1. Read error from descriptor.
+ *
+ *   2. Protocol error from the remote (e.g., bogus length characters).
+ *
+ *   3. Receiving a packet larger than "size" bytes.
+ *
+ *   4. Truncated output from the remote (e.g., we expected a packet but got
+ *      EOF, or we got a partial packet followed by EOF).
+ */
 int packet_read_line(int fd, char *buffer, unsigned size);
-int packet_read(int fd, char *buffer, unsigned size);
+
+/*
+ * Same as packet_read_line, but do not die on condition 4 (truncated input);
+ * instead return -1.  We still die for the other conditions.
+ */
+int packet_read_line_gently(int fd, char *buffer, unsigned size);
+
 int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
 ssize_t safe_write(int, const void *, ssize_t);
 
-- 
1.8.1.20.g7078b03