Odb: use a callback instead of a channel for ForEach

A channel provides no way to specify whether we stopped sending data
because of an error or because there is no more data.

Therefore, make Odb.ForEach() take a callback with which the user is free to
do whatever they need, letting us return en error.

odb.go

@@ -84,30 +84,40 @@ func (v *Odb) Read(oid *Oid) (obj *OdbObject, err error) {
 	return obj, nil
 }
 
+type OdbForEachCallback func(id *Oid) error
+
+type foreachData struct {
+	callback OdbForEachCallback
+	err error
+}
+
 //export odbForEachCb
 func odbForEachCb(id *C.git_oid, payload unsafe.Pointer) int {
-	ch := *(*chan *Oid)(payload)
+	data := (*foreachData)(payload)
-	oid := newOidFromC(id)
+
-	// Because the channel is unbuffered, we never read our own data. If ch is
+	err := data.callback(newOidFromC(id))
-	// readable, the user has sent something on it, which means we should
+	if err != nil {
-	// abort.
+		data.err = err
-	select {
+		return C.GIT_EUSER
-	case ch <- oid:
-	case <-ch:
-		return -1
 	}
+
 	return 0
 }
 
-func (v *Odb) forEachWrap(ch chan *Oid) {
+func (v *Odb) ForEach(callback OdbForEachCallback) error {
-	C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&ch))
+	data := foreachData {
-	close(ch)
+		callback: callback,
-}
+		err: nil,
+	}
 
-func (v *Odb) ForEach() chan *Oid {
+	ret := C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&data))
-	ch := make(chan *Oid, 0)
+	if ret == C.GIT_EUSER {
-	go v.forEachWrap(ch)
+		return data.err
-	return ch
+	} else if ret < 0 {
+		return MakeGitError(ret)
+	}
+
+	return nil
 }
 
 // Hash determines the object-ID (sha1) of a data buffer.

odb_test.go

@@ -3,6 +3,7 @@ package git
 import (
 	"io"
 	"os"
+	"errors"
 	"testing"
 )
 
@@ -48,7 +49,7 @@ parent 66e1c476199ebcd3e304659992233132c5a52c6c
 author John Doe <john@doe.com> 1390682018 +0000
 committer John Doe <john@doe.com> 1390682018 +0000
 
-Initial commit.`;
+Initial commit.`
 
 	oid, error := odb.Hash([]byte(str), ObjectCommit)
 	checkFatal(t, error)
@@ -60,3 +61,36 @@ Initial commit.`;
 		t.Fatal("Hash and write Oids are different")
 	}
 }
+
+func TestOdbForeach(t *testing.T) {
+	repo := createTestRepo(t)
+	defer os.RemoveAll(repo.Workdir())
+	_, _ = seedTestRepo(t, repo)
+
+	odb, err := repo.Odb()
+	checkFatal(t, err)
+
+	expect := 3
+	count := 0
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return nil
+	})
+
+	checkFatal(t, err)
+	if count != expect {
+		t.Fatalf("Expected %v objects, got %v")
+	}
+
+	expect = 1
+	count = 0
+	to_return := errors.New("not really an error")
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return to_return
+	})
+
+	if err != to_return {
+		t.Fatalf("Odb.ForEach() did not return the expected error, got %v", err)
+	}
+}