[PATCH 14/16] fetch-pack: refactor parsing in get_ack

From: Jeff King <peff@peff.net>
Date: Jun 18, 2014, 19:56 UTC
Message-ID: <20140618195603.GN22622@sigill.intra.peff.net>
In-Reply-To: <20140618194117.GA22269@sigill.intra.peff.net>

There are several uses of the magic number "line+45" when parsing ACK lines from the server, and it's rather unclear why 45 is the correct number. We can make this more clear by keeping a running pointer as we parse, using skip_prefix to jump past the first "ACK ", then adding 40 to jump past get_sha1_hex (which is still magical, but hopefully 40 is less magical to readers of git code).

Note that this actually puts us at line+44. The original required some character between the sha1 and further ACK flags (it is supposed to be a space, but we never enforced that). We start our search for flags at line+44, which meanas we are slightly more liberal than the old code.

Signed-off-by: Jeff King <peff@peff.net>
---
I actually think we could tighten this even more and drop the strstrs,
too, like:

  arg += 40;
  if (*arg++ != ' ')
	return ACK;
  if (!strcmp(arg, "continue"))
	return ACK_continue;

and so on. But I wasn't sure if there was a reason for the use of strstr. According to pack-protocol.txt, we would only get one at a time, and always with a single space between them.

 fetch-pack.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 3de3bd5..72ec520 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -189,20 +189,23 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 {
 	int len;
 	char *line = packet_read_line(fd, &len);
+	const char *arg;
 
 	if (!len)
 		die("git fetch-pack: expected ACK/NAK, got EOF");
 	if (!strcmp(line, "NAK"))
 		return NAK;
-	if (starts_with(line, "ACK ")) {
-		if (!get_sha1_hex(line+4, result_sha1)) {
-			if (len < 45)
+	if (skip_prefix(line, "ACK ", &arg)) {
+		if (!get_sha1_hex(arg, result_sha1)) {
+			arg += 40;
+			len -= arg - line;
+			if (len < 1)
 				return ACK;
-			if (strstr(line+45, "continue"))
+			if (strstr(arg, "continue"))
 				return ACK_continue;
-			if (strstr(line+45, "common"))
+			if (strstr(arg, "common"))
 				return ACK_common;
-			if (strstr(line+45, "ready"))
+			if (strstr(arg, "ready"))
 				return ACK_ready;
 			return ACK;
 		}

-- 
2.0.0.566.gfe3e6b2