Merge pull request #2640 from greatroar/simplify-loadblob-usage

Simplify Repository.LoadBlob usage

cmd/restic/cmd_cat.go

@@ -170,18 +170,15 @@ func runCat(gopts GlobalOptions, args []string) error {
 
 	case "blob":
 		for _, t := range []restic.BlobType{restic.DataBlob, restic.TreeBlob} {
-			list, found := repo.Index().Lookup(id, t)
+			_, found := repo.Index().Lookup(id, t)
 			if !found {
 				continue
 			}
-			blob := list[0]
 
-			buf := make([]byte, blob.Length)
+			buf, err := repo.LoadBlob(gopts.ctx, t, id, nil)
-			n, err := repo.LoadBlob(gopts.ctx, t, id, buf)
 			if err != nil {
 				return err
 			}
-			buf = buf[:n]
 
 			_, err = os.Stdout.Write(buf)
 			return err

cmd/restic/cmd_dump.go

@@ -171,24 +171,15 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error {
 }
 
 func getNodeData(ctx context.Context, output io.Writer, repo restic.Repository, node *restic.Node) error {
-	var buf []byte
+	var (
+		buf []byte
+		err error
+	)
 	for _, id := range node.Content {
-
+		buf, err = repo.LoadBlob(ctx, restic.DataBlob, id, buf)
-		size, found := repo.LookupBlobSize(id, restic.DataBlob)
-		if !found {
-			return errors.Errorf("id %v not found in repository", id)
-		}
-
-		buf = buf[:cap(buf)]
-		if len(buf) < restic.CiphertextLength(int(size)) {
-			buf = restic.NewBlobBuffer(int(size))
-		}
-
-		n, err := repo.LoadBlob(ctx, restic.DataBlob, id, buf)
 		if err != nil {
 			return err
 		}
-		buf = buf[:n]
 
 		_, err = output.Write(buf)
 		if err != nil {

internal/archiver/testing.go

@@ -228,13 +228,13 @@ func TestEnsureFileContent(ctx context.Context, t testing.TB, repo restic.Reposi
 	content := make([]byte, restic.CiphertextLength(len(file.Content)))
 	pos := 0
 	for _, id := range node.Content {
-		n, err := repo.LoadBlob(ctx, restic.DataBlob, id, content[pos:])
+		part, err := repo.LoadBlob(ctx, restic.DataBlob, id, content[pos:])
 		if err != nil {
 			t.Fatalf("error loading blob %v: %v", id.Str(), err)
 			return
 		}
 
-		pos += n
+		pos += len(part)
 	}
 
 	content = content[:pos]

internal/fuse/file.go

@@ -96,15 +96,14 @@ func (f *file) getBlobAt(ctx context.Context, i int) (blob []byte, err error) {
 		f.blobs[j] = nil
 	}
 
-	buf := restic.NewBlobBuffer(f.sizes[i])
+	blob, err = f.root.repo.LoadBlob(ctx, restic.DataBlob, f.node.Content[i], nil)
-	n, err := f.root.repo.LoadBlob(ctx, restic.DataBlob, f.node.Content[i], buf)
 	if err != nil {
 		debug.Log("LoadBlob(%v, %v) failed: %v", f.node.Name, f.node.Content[i], err)
 		return nil, err
 	}
-	f.blobs[i] = buf[:n]
+	f.blobs[i] = blob
 
-	return buf[:n], nil
+	return blob, nil
 }
 
 func (f *file) Read(ctx context.Context, req *fuse.ReadRequest, resp *fuse.ReadResponse) error {

internal/fuse/fuse_test.go

@@ -94,12 +94,11 @@ func TestFuseFile(t *testing.T) {
 		rtest.Assert(t, found, "Expected to find blob id %v", id)
 		filesize += uint64(size)
 
-		buf := restic.NewBlobBuffer(int(size))
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, nil)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		rtest.OK(t, err)
 
-		if uint(n) != size {
+		if len(buf) != int(size) {
-			t.Fatalf("not enough bytes read for id %v: want %v, got %v", id.Str(), size, n)
+			t.Fatalf("not enough bytes read for id %v: want %v, got %v", id.Str(), size, len(buf))
 		}
 
 		if uint(len(buf)) != size {

internal/repository/repository.go

@@ -130,17 +130,16 @@ func (r *Repository) sortCachedPacks(blobs []restic.PackedBlob) []restic.PackedB
 	return append(cached, noncached...)
 }
 
-// loadBlob tries to load and decrypt content identified by t and id from a
+// LoadBlob loads a blob of type t from the repository.
-// pack from the backend, the result is stored in plaintextBuf, which must be
+// It may use all of buf[:cap(buf)] as scratch space.
-// large enough to hold the complete blob.
+func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic.ID, buf []byte) ([]byte, error) {
-func (r *Repository) loadBlob(ctx context.Context, id restic.ID, t restic.BlobType, plaintextBuf []byte) (int, error) {
+	debug.Log("load %v with id %v (buf len %v, cap %d)", t, id, len(buf), cap(buf))
-	debug.Log("load %v with id %v (buf len %v, cap %d)", t, id, len(plaintextBuf), cap(plaintextBuf))
 
 	// lookup packs
 	blobs, found := r.idx.Lookup(id, t)
 	if !found {
 		debug.Log("id %v not found in index", id)
-		return 0, errors.Errorf("id %v not found in repository", id)
+		return nil, errors.Errorf("id %v not found in repository", id)
 	}
 
 	// try cached pack files first
@@ -157,13 +156,14 @@ func (r *Repository) loadBlob(ctx context.Context, id restic.ID, t restic.BlobTy
 		// load blob from pack
 		h := restic.Handle{Type: restic.DataFile, Name: blob.PackID.String()}
 
-		if uint(cap(plaintextBuf)) < blob.Length {
+		switch {
-			return 0, errors.Errorf("buffer is too small: %v < %v", cap(plaintextBuf), blob.Length)
+		case cap(buf) < int(blob.Length):
+			buf = make([]byte, blob.Length)
+		case len(buf) != int(blob.Length):
+			buf = buf[:blob.Length]
 		}
 
-		plaintextBuf = plaintextBuf[:blob.Length]
+		n, err := restic.ReadAt(ctx, r.be, h, int64(blob.Offset), buf)
-
-		n, err := restic.ReadAt(ctx, r.be, h, int64(blob.Offset), plaintextBuf)
 		if err != nil {
 			debug.Log("error loading blob %v: %v", blob, err)
 			lastError = err
@@ -178,7 +178,7 @@ func (r *Repository) loadBlob(ctx context.Context, id restic.ID, t restic.BlobTy
 		}
 
 		// decrypt
-		nonce, ciphertext := plaintextBuf[:r.key.NonceSize()], plaintextBuf[r.key.NonceSize():]
+		nonce, ciphertext := buf[:r.key.NonceSize()], buf[r.key.NonceSize():]
 		plaintext, err := r.key.Open(ciphertext[:0], nonce, ciphertext, nil)
 		if err != nil {
 			lastError = errors.Errorf("decrypting blob %v failed: %v", id, err)
@@ -191,16 +191,16 @@ func (r *Repository) loadBlob(ctx context.Context, id restic.ID, t restic.BlobTy
 			continue
 		}
 
-		// move decrypted data to the start of the provided buffer
+		// move decrypted data to the start of the buffer
-		copy(plaintextBuf[0:], plaintext)
+		copy(buf, plaintext)
-		return len(plaintext), nil
+		return buf[:len(plaintext)], nil
 	}
 
 	if lastError != nil {
-		return 0, lastError
+		return nil, lastError
 	}
 
-	return 0, errors.Errorf("loading blob %v from %v packs failed", id.Str(), len(blobs))
+	return nil, errors.Errorf("loading blob %v from %v packs failed", id.Str(), len(blobs))
 }
 
 // LoadJSONUnpacked decrypts the data and afterwards calls json.Unmarshal on
@@ -670,31 +670,6 @@ func (r *Repository) Close() error {
 	return r.be.Close()
 }
 
-// LoadBlob loads a blob of type t from the repository to the buffer. buf must
-// be large enough to hold the encrypted blob, since it is used as scratch
-// space.
-func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic.ID, buf []byte) (int, error) {
-	debug.Log("load blob %v into buf (len %v, cap %v)", id, len(buf), cap(buf))
-	size, found := r.idx.LookupSize(id, t)
-	if !found {
-		return 0, errors.Errorf("id %v not found in repository", id)
-	}
-
-	if cap(buf) < restic.CiphertextLength(int(size)) {
-		return 0, errors.Errorf("buffer is too small for data blob (%d < %d)", cap(buf), restic.CiphertextLength(int(size)))
-	}
-
-	n, err := r.loadBlob(ctx, id, t, buf)
-	if err != nil {
-		return 0, err
-	}
-	buf = buf[:n]
-
-	debug.Log("loaded %d bytes into buf %p", len(buf), buf)
-
-	return len(buf), err
-}
-
 // SaveBlob saves a blob of type t into the repository. If id is the null id, it
 // will be computed and returned.
 func (r *Repository) SaveBlob(ctx context.Context, t restic.BlobType, buf []byte, id restic.ID) (restic.ID, error) {
@@ -709,19 +684,10 @@ func (r *Repository) SaveBlob(ctx context.Context, t restic.BlobType, buf []byte
 func (r *Repository) LoadTree(ctx context.Context, id restic.ID) (*restic.Tree, error) {
 	debug.Log("load tree %v", id)
 
-	size, found := r.idx.LookupSize(id, restic.TreeBlob)
+	buf, err := r.LoadBlob(ctx, restic.TreeBlob, id, nil)
-	if !found {
-		return nil, errors.Errorf("tree %v not found in repository", id)
-	}
-
-	debug.Log("size is %d, create buffer", size)
-	buf := restic.NewBlobBuffer(int(size))
-
-	n, err := r.loadBlob(ctx, id, restic.TreeBlob, buf)
 	if err != nil {
 		return nil, err
 	}
-	buf = buf[:n]
 
 	t := &restic.Tree{}
 	err = json.Unmarshal(buf, t)

internal/repository/repository_test.go

@@ -43,10 +43,9 @@ func TestSave(t *testing.T) {
 		// rtest.OK(t, repo.SaveIndex())
 
 		// read back
-		buf := restic.NewBlobBuffer(size)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, nil)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		rtest.OK(t, err)
-		rtest.Equals(t, len(buf), n)
+		rtest.Equals(t, size, len(buf))
 
 		rtest.Assert(t, len(buf) == len(data),
 			"number of bytes read back does not match: expected %d, got %d",
@@ -77,10 +76,9 @@ func TestSaveFrom(t *testing.T) {
 		rtest.OK(t, repo.Flush(context.Background()))
 
 		// read back
-		buf := restic.NewBlobBuffer(size)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, nil)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		rtest.OK(t, err)
-		rtest.Equals(t, len(buf), n)
+		rtest.Equals(t, size, len(buf))
 
 		rtest.Assert(t, len(buf) == len(data),
 			"number of bytes read back does not match: expected %d, got %d",
@@ -164,33 +162,17 @@ func TestLoadBlob(t *testing.T) {
 	rtest.OK(t, err)
 	rtest.OK(t, repo.Flush(context.Background()))
 
-	// first, test with buffers that are too small
-	for _, testlength := range []int{length - 20, length, restic.CiphertextLength(length) - 1} {
-		buf = make([]byte, 0, testlength)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
-		if err == nil {
-			t.Errorf("LoadBlob() did not return an error for a buffer that is too small to hold the blob")
-			continue
-		}
-
-		if n != 0 {
-			t.Errorf("LoadBlob() returned an error and n > 0")
-			continue
-		}
-	}
-
-	// then use buffers that are large enough
 	base := restic.CiphertextLength(length)
-	for _, testlength := range []int{base, base + 7, base + 15, base + 1000} {
+	for _, testlength := range []int{0, base - 20, base - 1, base, base + 7, base + 15, base + 1000} {
 		buf = make([]byte, 0, testlength)
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		buf, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		if err != nil {
 			t.Errorf("LoadBlob() returned an error for buffer size %v: %v", testlength, err)
 			continue
 		}
 
-		if n != length {
+		if len(buf) != length {
-			t.Errorf("LoadBlob() returned the wrong number of bytes: want %v, got %v", length, n)
+			t.Errorf("LoadBlob() returned the wrong number of bytes: want %v, got %v", length, len(buf))
 			continue
 		}
 	}
@@ -213,13 +195,14 @@ func BenchmarkLoadBlob(b *testing.B) {
 	b.SetBytes(int64(length))
 
 	for i := 0; i < b.N; i++ {
-		n, err := repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
+		var err error
+		buf, err = repo.LoadBlob(context.TODO(), restic.DataBlob, id, buf)
 		rtest.OK(b, err)
-		if n != length {
+		if len(buf) != length {
-			b.Errorf("wanted %d bytes, got %d", length, n)
+			b.Errorf("wanted %d bytes, got %d", length, len(buf))
 		}
 
-		id2 := restic.Hash(buf[:n])
+		id2 := restic.Hash(buf)
 		if !id.Equal(id2) {
 			b.Errorf("wrong data returned, wanted %v, got %v", id.Str(), id2.Str())
 		}

internal/restic/node.go

@@ -282,21 +282,10 @@ func (node Node) createFileAt(ctx context.Context, path string, repo Repository)
 func (node Node) writeNodeContent(ctx context.Context, repo Repository, f *os.File) error {
 	var buf []byte
 	for _, id := range node.Content {
-		size, found := repo.LookupBlobSize(id, DataBlob)
+		buf, err := repo.LoadBlob(ctx, DataBlob, id, buf)
-		if !found {
-			return errors.Errorf("id %v not found in repository", id)
-		}
-
-		buf = buf[:cap(buf)]
-		if len(buf) < CiphertextLength(int(size)) {
-			buf = NewBlobBuffer(int(size))
-		}
-
-		n, err := repo.LoadBlob(ctx, DataBlob, id, buf)
 		if err != nil {
 			return err
 		}
-		buf = buf[:n]
 
 		_, err = f.Write(buf)
 		if err != nil {

internal/restic/repository.go

@@ -45,7 +45,7 @@ type Repository interface {
 	// new buffer will be allocated and returned.
 	LoadAndDecrypt(ctx context.Context, buf []byte, t FileType, id ID) (data []byte, err error)
 
-	LoadBlob(context.Context, BlobType, ID, []byte) (int, error)
+	LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error)
 	SaveBlob(context.Context, BlobType, []byte, ID) (ID, error)
 
 	LoadTree(context.Context, ID) (*Tree, error)