Re: [PATCH v2 39/53] refs/files-backend: convert many internals to struct object_id
From: Jonathan Tan <hidden>
Date: 2017-05-01 23:24:31
On 04/30/2017 07:29 PM, brian m. carlson wrote:
quoted hunk ↗ jump to hunk
@@ -195,27 +195,18 @@ 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(struct strbuf *line, unsigned char *sha1) +static const char *parse_ref_line(struct strbuf *line, struct object_id *oid) { const char *ref; - /* - * 42: the answer to everything. - * - * In this case, it happens to be the answer to - * 40 (length of sha1 hex representation) - * +1 (space in between hex and name) - * +1 (newline at the end of the line) - */ - if (line->len <= 42) + if (!line->len) return NULL;
I would omit this check - parse_oid_hex already exits early if the first character is NUL. (The existing code makes a bit more sense, in that it avoids checking the first few characters if we already know a bit more about the string.)
- if (get_sha1_hex(line->buf, sha1) < 0) + if (parse_oid_hex(line->buf, oid, &ref) < 0) return NULL; - if (!isspace(line->buf[40])) + if (!isspace(*ref++)) return NULL; - ref = line->buf + 41; if (isspace(*ref)) return NULL;
Looks fine, up to here. (Also, you requested extra-careful review for this patch, but this patch seems mostly mechanical to me.)