Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:42:33
Hi, On Tue, 11 Jul 2006, Linus Torvalds wrote:
quoted hunk ↗ jump to hunk
@@ -762,20 +765,65 @@ static int parse_sha1_header(char *hdr, /* * The length must be followed by a zero byte */ - return *hdr ? -1 : 0; + bytes++; + if (*hdr) + bytes = -1; + return bytes;
Why not just say return *hdr ? -1 : bytes;
+}
+
+static int parse_binary_sha1_header(char *hdr, char *type, unsigned long *sizep)
+{
+ unsigned char c;
+ int bytes = 1;
+ unsigned long size;
+ unsigned object_type, bits;
+ static const char *typename[8] = {
+ NULL, /* OBJ_EXT */
+ "commit", "tree", "blob", "tag",
+ NULL, NULL, NULL
+ };
+
+ c = *hdr++;
+ object_type = (c >> 4) & 7;
+ if (!typename[object_type])
+ return -1;You might want to add a comment saying "since the type is lowercase in ascii format, object_type must be 6 or 7, which is an invalid object type." It took me a little to figure that out...
+ strcpy(type, typename[object_type]); + size = c & 15;
Just a nit here: I think 0xf is easier to read with boolean operations.
+ bits = 4;
+ while (!(c & 0x80)) {
+ if (bits >= 8*sizeof(unsigned long))
+ return -1;
+ c = *hdr++;
+ size += (unsigned long) (c & 0x7f) << bits;
+ bytes++;
+ bits += 7;
+ }Are you not losing the last byte by putting the "while" _before_ instead of _after_ the loop?
quoted hunk ↗ jump to hunk
@@ -1192,7 +1240,7 @@ struct packed_git *find_sha1_pack(const int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep) { - int status; + int status, hdrlen; unsigned long mapsize, size; void *map; z_stream stream; -
This hunk is unnecessary, right? Ciao, Dscho