Fix bug with OFS objects with length < 128 bytes

* Also remove unnecessary logging statements

Author: ChimeraCoder

delta.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"log"
 	"os"
 )
 
@@ -15,9 +14,9 @@ func patchDelta(start io.ReadSeeker, delta io.Reader) (io.Reader, error) {
 
 	// First, read the source and target lengths (varints)
 	// we can ignore err as long as we check deltar.err at the end
+	// we have no need for the targetLength right now, so we discard it
 	sourceLength, _ := parseVarInt(deltar)
-	targetLength, _ := parseVarInt(deltar)
-	log.Printf("Expecting target length %d", targetLength)
+	_, _ = parseVarInt(deltar)
 	if deltar.err != nil {
 		return nil, deltar.err
 	}
@@ -37,7 +36,6 @@ func patchDelta(start io.ReadSeeker, delta io.Reader) (io.Reader, error) {
 
 		// b represents our command itself (opcode)
 		b := bs[0]
-		log.Printf("b is %08b", b)
 		switch b & 128 {
 		case 128:
 			// b is a copy instruction
@@ -64,29 +62,24 @@ func patchDelta(start io.ReadSeeker, delta io.Reader) (io.Reader, error) {
 			// if the fifth bit from the right is set, read the next byte
 			// The number of copy bytes must fit into
 
-			var numBytes uint = 0
+			var numBytes uint
 
 			if (b & 16) > 0 {
 				numBytes = numBytes | uint(uint(deltar.readByte(true)))
-				log.Printf("16 %d", numBytes)
 			}
 			if (b & 32) > 0 {
 				numBytes = numBytes | uint((uint(deltar.readByte(true)) << 8))
-				log.Printf("32 %d", numBytes)
 			}
 
 			if (b & 64) > 0 {
 				numBytes = numBytes | uint((uint(deltar.readByte(true)) << 16))
-				log.Printf("64 %d", numBytes)
 			}
 
 			// Default to 0x10000 due to overflow
 			if numBytes == 0 {
 				numBytes = 65536
 			}
 
-			log.Printf("copy offset %d", baseOffset)
-			log.Printf("copy length %d", numBytes)
 			// read numBytes from source, starting at baseOffset
 			// and write that to the target
 			base.Seek(int64(baseOffset), os.SEEK_SET)
@@ -112,7 +105,6 @@ func patchDelta(start io.ReadSeeker, delta io.Reader) (io.Reader, error) {
 
 			numBytes := int(b)
 			buf := make([]byte, numBytes)
-			log.Printf("Inserting %d", numBytes)
 			deltar.read(buf)
 			_, err := result.Write(buf)
 			if err != nil {
@@ -219,9 +211,6 @@ func (er *errReader) readByte(p ...bool) byte {
 		}
 		er.err = fmt.Errorf("expected to read single byte and read none")
 	}
-	if len(p) != 0 {
-		log.Printf("Read %d", b[0])
-	}
 	return b[0]
 }
 

delta_test.go

@@ -67,7 +67,6 @@ func Test_PatchDelta(t *testing.T) {
 			t.Errorf("delta application failed")
 			continue
 		}
-		log.Printf("SUCCESS")
 	}
 }
 

verify-pack.go

@@ -72,22 +72,23 @@ func VerifyPack(pack io.ReadSeeker, idx io.Reader) ([]*packObject, error) {
 	objects, err := parsePack(errReadSeeker{pack, nil}, idx)
 	for _, object := range objects {
 		if object.err != nil {
-			log.Printf("Found %s (%d) %s %s", object.Name, object.size, object.Type, object.err)
+			log.Printf("Found %s (%d) %s", object.Name, object.size, object.err)
 		}
 
-		if object.Type == OBJ_OFS_DELTA {
-			var base *packObject
+		if object.Type == OBJ_OFS_DELTA && object.err != nil {
 			// TODO improve this
 			// linear search to find the right offset
+			var base *packObject
 			for _, o := range objects {
-				log.Print(o.Offset)
 				if o.Offset == object.Offset-object.negativeOffset {
 					base = o
 					break
 				}
 			}
 			if base == nil {
-				return nil, fmt.Errorf("Could not find object with negative offset %d - %d", object.Offset, object.negativeOffset)
+				object.err = fmt.Errorf("could not find object with negative offset %d - %d for %s", object.Offset, object.negativeOffset, object.Name)
+			} else {
+				log.Printf("SUCCESS with %s", object.Name)
 			}
 		}
 
@@ -104,13 +105,10 @@ func parsePack(pack errReadSeeker, idx io.Reader) (objects []*packObject, err er
 	if err != nil {
 		return nil, err
 	}
-	log.Printf("signature %+v", signature)
-
 	version := make([]byte, 4)
 	pack.read(version)
 
 	// TODO use encoding/binary here
-	log.Printf("version is %+v", version)
 	v := version[3]
 	switch v {
 	case 2:
@@ -125,8 +123,6 @@ func parsePack(pack errReadSeeker, idx io.Reader) (objects []*packObject, err er
 	default:
 		return nil, fmt.Errorf("cannot parse packfile with version %d", v)
 	}
-
-	return nil, nil
 }
 
 func Clone(r io.Reader) (*bufio.Reader, *bufio.Reader) {
@@ -156,6 +152,7 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 	}
 
 	for _, object := range objects {
+
 		var btsread int
 		r.Seek(int64(object.Offset), os.SEEK_SET)
 		_bytes := make([]byte, 1)
@@ -210,6 +207,7 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 			if err != nil {
 				return nil, err
 			}
+
 			n, err := zr.Read(object.Data)
 			if err != nil {
 				if err == io.EOF {
@@ -233,12 +231,11 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 			// for n >= 2 adding 2^7 + 2^14 + ... + 2^(7*(n-1))
 			// to the result."
 
-			log.Printf("encountered ofs delta")
-
 			var offset int
 			MSB := (_byte & 128) // will be either 128 or 0
 			var shift uint = 0
-			for MSB > 0 {
+
+			for {
 				// Keep reading the size until the MSB is 0
 				_bytes := make([]byte, 1)
 				r.read(_bytes)
@@ -248,14 +245,16 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 
 				offset += int((uint(_byte) & 127) << shift)
 				shift += 7
+
+				if MSB == 0 {
+					break
+				}
 			}
 			object.negativeOffset = offset
 			object.Data = make([]byte, objectSize)
 
 			zr, err := zlib.NewReader(r.r)
 			if err != nil {
-				log.Printf("Shift is %d", shift)
-				log.Printf("Object size is %d", objectSize)
 				object.err = err
 				continue
 			}
@@ -266,7 +265,9 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 			}
 			object.Data = object.Data[:n]
 			zr.Close()
-			log.Printf("Ofs data is %q", object.Data)
+			if len(object.Data) != objectSize {
+				object.err = fmt.Errorf("received wrong object size: %d (expected %d)", object.Data, objectSize)
+			}
 
 		case object.Type == OBJ_REF_DELTA:
 			r.Seek(int64(object.Offset), os.SEEK_SET)
@@ -296,7 +297,7 @@ func parsePackV2(r errReadSeeker, objects []*packObject) ([]*packObject, error)
 
 func parseIdx(idx io.Reader, version int) (objects []*packObject, err error) {
 	if version != 2 {
-		return nil, fmt.Errorf("cannot parse IDX with version %d")
+		return nil, fmt.Errorf("cannot parse IDX with version %d", version)
 	}
 	// parse version 2 idxfile