Re: [PATCH v2 39/53] refs/files-backend: convert many internals to struct object_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]