From ae021d87911da4328157273df24779892cb51277 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Jun 2014 15:47:50 -0400 Subject: use skip_prefix to avoid magic numbers It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fetch-pack.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'fetch-pack.c') diff --git a/fetch-pack.c b/fetch-pack.c index eeee2bb7e0..3de3bd508e 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -319,18 +319,19 @@ static int find_common(struct fetch_pack_args *args, if (args->depth > 0) { char *line; + const char *arg; unsigned char sha1[20]; send_request(args, fd[1], &req_buf); while ((line = packet_read_line(fd[0], NULL))) { - if (starts_with(line, "shallow ")) { - if (get_sha1_hex(line + 8, sha1)) + if (skip_prefix(line, "shallow ", &arg)) { + if (get_sha1_hex(arg, sha1)) die("invalid shallow line: %s", line); register_shallow(sha1); continue; } - if (starts_with(line, "unshallow ")) { - if (get_sha1_hex(line + 10, sha1)) + if (skip_prefix(line, "unshallow ", &arg)) { + if (get_sha1_hex(arg, sha1)) die("invalid unshallow line: %s", line); if (!lookup_object(sha1)) die("object not found: %s", line); -- cgit v1.2.1 From 82e56767aa9334bb0c4cfe81964a576778b93d6e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Jun 2014 15:56:03 -0400 Subject: fetch-pack: refactor parsing in get_ack 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 Signed-off-by: Junio C Hamano --- fetch-pack.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'fetch-pack.c') diff --git a/fetch-pack.c b/fetch-pack.c index 3de3bd508e..72ec520fda 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; } -- cgit v1.2.1