On Tue, 11 Jul 2006, Johannes Schindelin wrote: > > Why not just say > > return *hdr ? -1 : bytes; Hey, whatever works. I rewrote more, and edited some of my changes down again.. > 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... It's not even correct in my version - I check the ASCII header _first_, so by the time it looks at the binary one, it already knows it's not ascii. The problematic case is actually the other way around: my "parse_ascii_sha1_header()" isn't strict enough. Or, more likely, the parse_sha1_header() function should just be changed to check the binary format first (and then add your comment about why that is safe). > > + 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? No. The very first byte can have the 0x80 end marker, when the size was between 0..15. > > 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? Yeah, never mind. That function didn't actually need the hdrlen, it only cared about the SHA1. Linus - : send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html