Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"

Peter Baumann [off-list ref]
writes:

quoted

+	bits = 4;
+	size = c & 0xf;
+	while (!(c & 0x80)) {
+		if (bits >= 8*sizeof(long))
+			return -1;
+		c = *map++;
+		size += (c & 0x7f) << bits;
+		bits += 7;
+		mapsize--;
+	}

This doesn't match the logic used in unpack_object_header, which is used
in the packs:
...

quoted

+	c = (type << 4) | (len & 15);
+	len >>= 4;
+	hdr_len = 1;
+	while (len) {
+		*hdr++ = c;
+		hdr_len++;
+		c = (len & 0x7f);
+		len >>= 7;
+	}
+	*hdr = c | 0x80;
+	return hdr_len;
+}
+

Dito, but in this case see pack-objects.c

Well, while these are not strictly needed to match, there is no
good reason to make them inconsistent.  Very well spotted.

On Wed, Jul 12, 2006 at 12:16:12AM -0700, Junio C Hamano wrote:

quoted

quoted

+	bits = 4;
+	size = c & 0xf;
+	while (!(c & 0x80)) {
+		if (bits >= 8*sizeof(long))
+			return -1;
+		c = *map++;
+		size += (c & 0x7f) << bits;
+		bits += 7;
+		mapsize--;
+	}

This doesn't match the logic used in unpack_object_header, which is used
in the packs:
...

quoted

+	c = (type << 4) | (len & 15);
+	len >>= 4;
+	hdr_len = 1;
+	while (len) {
+		*hdr++ = c;
+		hdr_len++;
+		c = (len & 0x7f);
+		len >>= 7;
+	}
+	*hdr = c | 0x80;
+	return hdr_len;
+}
+

Dito, but in this case see pack-objects.c

Well, while these are not strictly needed to match, there is no
good reason to make them inconsistent.  Very well spotted.

During packing one could simply copy the existing object into the generated
pack in the non delta case, so I think this _is_ necessary/usefull.

-Peter Baumann