On 04/30/2017 07:29 PM, brian m. carlson wrote:
@@ -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.)

Reply via email to