From 10c497aa0cc7ccb5bd213f53fc0cd803bd73a508 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Dec 2014 05:40:07 -0500 Subject: read_packed_refs: use a strbuf for reading lines Current code uses a fixed PATH_MAX-sized buffer for reading packed-refs lines. This is a reasonable guess, in the sense that git generally cannot work with refs larger than PATH_MAX. However, there are a few cases where it is not great: 1. Some systems may have a low value of PATH_MAX, but can actually handle larger paths in practice. Fixing this code path probably isn't enough to make them work completely with long refs, but it is a step in the right direction. 2. We use fgets, which will happily give us half a line on the first read, and then the rest of the line on the second. This is probably OK in practice, because our refline parser is careful enough to look for the trailing newline on the first line. The second line may look like a peeled line to us, but since "^" is illegal in refnames, it is not likely to come up. Still, it does not hurt to be more careful. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 82e5b1b..60a7e33 100644 --- a/refs.c +++ b/refs.c @@ -1031,16 +1031,16 @@ static const char *parse_ref_line(char *line, unsigned char *sha1) static void read_packed_refs(FILE *f, struct ref_dir *dir) { struct ref_entry *last = NULL; - char refline[PATH_MAX]; + struct strbuf line = STRBUF_INIT; enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE; - while (fgets(refline, sizeof(refline), f)) { + while (strbuf_getwholeline(&line, f, '\n') != EOF) { unsigned char sha1[20]; const char *refname; static const char header[] = "# pack-refs with:"; - if (!strncmp(refline, header, sizeof(header)-1)) { - const char *traits = refline + sizeof(header) - 1; + if (!strncmp(line.buf, header, sizeof(header)-1)) { + const char *traits = line.buf + sizeof(header) - 1; if (strstr(traits, " fully-peeled ")) peeled = PEELED_FULLY; else if (strstr(traits, " peeled ")) @@ -1049,7 +1049,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) continue; } - refname = parse_ref_line(refline, sha1); + refname = parse_ref_line(line.buf, sha1); if (refname) { last = create_ref_entry(refname, sha1, REF_ISPACKED, 1); if (peeled == PEELED_FULLY || @@ -1059,10 +1059,10 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) continue; } if (last && - refline[0] == '^' && - strlen(refline) == PEELED_LINE_LENGTH && - refline[PEELED_LINE_LENGTH - 1] == '\n' && - !get_sha1_hex(refline + 1, sha1)) { + line.buf[0] == '^' && + line.len == PEELED_LINE_LENGTH && + line.buf[PEELED_LINE_LENGTH - 1] == '\n' && + !get_sha1_hex(line.buf + 1, sha1)) { hashcpy(last->u.value.peeled, sha1); /* * Regardless of what the file header said, @@ -1072,6 +1072,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) last->flag |= REF_KNOWS_PEELED; } } + + strbuf_release(&line); } /* -- cgit v0.10.2-6-g49f6 From 6a49870a7245934eac671e3dcd7186a90dac255d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Dec 2014 05:40:19 -0500 Subject: read_packed_refs: pass strbuf to parse_ref_line Now that we have a strbuf in read_packed_refs, we can pass it straight to the line parser, which saves us an extra strlen. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 60a7e33..047353e 100644 --- a/refs.c +++ b/refs.c @@ -973,8 +973,10 @@ static const char PACKED_REFS_HEADER[] = * Return a pointer to the refname within the line (null-terminated), * or NULL if there was a problem. */ -static const char *parse_ref_line(char *line, unsigned char *sha1) +static const char *parse_ref_line(struct strbuf *line, unsigned char *sha1) { + const char *ref; + /* * 42: the answer to everything. * @@ -983,22 +985,23 @@ static const char *parse_ref_line(char *line, unsigned char *sha1) * +1 (space in between hex and name) * +1 (newline at the end of the line) */ - int len = strlen(line) - 42; - - if (len <= 0) + if (line->len <= 42) return NULL; - if (get_sha1_hex(line, sha1) < 0) + + if (get_sha1_hex(line->buf, sha1) < 0) return NULL; - if (!isspace(line[40])) + if (!isspace(line->buf[40])) return NULL; - line += 41; - if (isspace(*line)) + + ref = line->buf + 41; + if (isspace(*ref)) return NULL; - if (line[len] != '\n') + + if (line->buf[line->len - 1] != '\n') return NULL; - line[len] = 0; + line->buf[--line->len] = 0; - return line; + return ref; } /* @@ -1049,7 +1052,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) continue; } - refname = parse_ref_line(line.buf, sha1); + refname = parse_ref_line(&line, sha1); if (refname) { last = create_ref_entry(refname, sha1, REF_ISPACKED, 1); if (peeled == PEELED_FULLY || -- cgit v0.10.2-6-g49f6 From ea417833ea9a07d7b8d568358ff20f57851cb26e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Dec 2014 05:40:36 -0500 Subject: read_packed_refs: use skip_prefix instead of static array We want to recognize the packed-refs header and skip to the "traits" part of the line. We currently do it by feeding sizeof() a static const array to strncmp. However, it's a bit simpler to just skip_prefix, which expresses the intention more directly, and without remembering to account for the NUL-terminator in each sizeof() call. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 047353e..ba3003f 100644 --- a/refs.c +++ b/refs.c @@ -1040,10 +1040,9 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) while (strbuf_getwholeline(&line, f, '\n') != EOF) { unsigned char sha1[20]; const char *refname; - static const char header[] = "# pack-refs with:"; + const char *traits; - if (!strncmp(line.buf, header, sizeof(header)-1)) { - const char *traits = line.buf + sizeof(header) - 1; + if (skip_prefix(line.buf, "# pack-refs with:", &traits)) { if (strstr(traits, " fully-peeled ")) peeled = PEELED_FULLY; else if (strstr(traits, " peeled ")) -- cgit v0.10.2-6-g49f6