Refactor all callbacks (#700)

This change is a preparation for another change that makes all callback
types return a Go error instead of an error code / an integer. That is
going to make make things a lot more idiomatic.

The reason this change is split is threefold:

a) This change is mostly mechanical and should contain no semantic
   changes.
b) This change is backwards-compatible (in the Go API compatibility
   sense of the word), and thus can be backported to all other releases.
c) It makes the other change a bit smaller and more focused on just one
   thing.

Concretely, this change makes all callbacks populate a Go error when
they fail. If the callback is invoked from the same stack as the
function to which it was passed (e.g. for `Tree.Walk`), it will preserve
the error object directly into a struct that also holds the callback
function. Otherwise if the callback is pased to one func and will be
invoked when run from another one (e.g. for `Repository.InitRebase`),
the error string is saved into the libgit2 thread-local storage and then
re-created as a `GitError`.

(cherry picked from commit 5d8eaf7e65)

checkout.go

@@ -3,10 +3,11 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_populate_checkout_cb(git_checkout_options *opts);
+extern void _go_git_populate_checkout_callbacks(git_checkout_options *opts);
 */
 import "C"
 import (
+	"errors"
 	"os"
 	"runtime"
 	"unsafe"
@@ -75,10 +76,10 @@ func checkoutOptionsFromC(c *C.git_checkout_options) CheckoutOptions {
 		NotifyFlags:    CheckoutNotifyType(c.notify_flags),
 	}
 	if c.notify_payload != nil {
-		opts.NotifyCallback = pointerHandles.Get(c.notify_payload).(*CheckoutOptions).NotifyCallback
+		opts.NotifyCallback = pointerHandles.Get(c.notify_payload).(*checkoutCallbackData).options.NotifyCallback
 	}
 	if c.progress_payload != nil {
-		opts.ProgressCallback = pointerHandles.Get(c.progress_payload).(*CheckoutOptions).ProgressCallback
+		opts.ProgressCallback = pointerHandles.Get(c.progress_payload).(*checkoutCallbackData).options.ProgressCallback
 	}
 	if c.target_directory != nil {
 		opts.TargetDirectory = C.GoString(c.target_directory)
@@ -86,19 +87,26 @@ func checkoutOptionsFromC(c *C.git_checkout_options) CheckoutOptions {
 	return opts
 }
 
-func (opts *CheckoutOptions) toC() *C.git_checkout_options {
+func (opts *CheckoutOptions) toC(errorTarget *error) *C.git_checkout_options {
 	if opts == nil {
 		return nil
 	}
-	c := C.git_checkout_options{}
+	return populateCheckoutOptions(&C.git_checkout_options{}, opts, errorTarget)
-	populateCheckoutOptions(&c, opts)
+}
-	return &c
+
+type checkoutCallbackData struct {
+	options     *CheckoutOptions
+	errorTarget *error
 }
 
 //export checkoutNotifyCallback
-func checkoutNotifyCallback(why C.git_checkout_notify_t, cpath *C.char, cbaseline, ctarget, cworkdir, data unsafe.Pointer) int {
+func checkoutNotifyCallback(
-	if data == nil {
+	why C.git_checkout_notify_t,
-		return 0
+	cpath *C.char,
+	cbaseline, ctarget, cworkdir, handle unsafe.Pointer,
+) C.int {
+	if handle == nil {
+		return C.int(ErrorCodeOK)
 	}
 	path := C.GoString(cpath)
 	var baseline, target, workdir DiffFile
@@ -111,26 +119,35 @@ func checkoutNotifyCallback(why C.git_checkout_notify_t, cpath *C.char, cbaselin
 	if cworkdir != nil {
 		workdir = diffFileFromC((*C.git_diff_file)(cworkdir))
 	}
-	opts := pointerHandles.Get(data).(*CheckoutOptions)
+	data := pointerHandles.Get(handle).(*checkoutCallbackData)
-	if opts.NotifyCallback == nil {
+	if data.options.NotifyCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
-	return int(opts.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir))
+	ret := data.options.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir)
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export checkoutProgressCallback
-func checkoutProgressCallback(path *C.char, completed_steps, total_steps C.size_t, data unsafe.Pointer) int {
+func checkoutProgressCallback(
-	opts := pointerHandles.Get(data).(*CheckoutOptions)
+	path *C.char,
-	if opts.ProgressCallback == nil {
+	completed_steps, total_steps C.size_t,
-		return 0
+	handle unsafe.Pointer,
+) {
+	data := pointerHandles.Get(handle).(*checkoutCallbackData)
+	if data.options.ProgressCallback == nil {
+		return
 	}
-	return int(opts.ProgressCallback(C.GoString(path), uint(completed_steps), uint(total_steps)))
+	data.options.ProgressCallback(C.GoString(path), uint(completed_steps), uint(total_steps))
 }
 
 // Convert the CheckoutOptions struct to the corresponding
 // C-struct. Returns a pointer to ptr, or nil if opts is nil, in order
 // to help with what to pass.
-func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions) *C.git_checkout_options {
+func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions, errorTarget *error) *C.git_checkout_options {
 	if opts == nil {
 		return nil
 	}
@@ -142,14 +159,18 @@ func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions)
 	ptr.file_mode = C.uint(opts.FileMode.Perm())
 	ptr.notify_flags = C.uint(opts.NotifyFlags)
 	if opts.NotifyCallback != nil || opts.ProgressCallback != nil {
-		C._go_git_populate_checkout_cb(ptr)
+		C._go_git_populate_checkout_callbacks(ptr)
-	}
+		data := &checkoutCallbackData{
-	payload := pointerHandles.Track(opts)
+			options:     opts,
-	if opts.NotifyCallback != nil {
+			errorTarget: errorTarget,
-		ptr.notify_payload = payload
+		}
-	}
+		payload := pointerHandles.Track(data)
-	if opts.ProgressCallback != nil {
+		if opts.NotifyCallback != nil {
-		ptr.progress_payload = payload
+			ptr.notify_payload = payload
+		}
+		if opts.ProgressCallback != nil {
+			ptr.progress_payload = payload
+		}
 	}
 	if opts.TargetDirectory != "" {
 		ptr.target_directory = C.CString(opts.TargetDirectory)
@@ -176,6 +197,8 @@ func freeCheckoutOptions(ptr *C.git_checkout_options) {
 	}
 	if ptr.notify_payload != nil {
 		pointerHandles.Untrack(ptr.notify_payload)
+	} else if ptr.progress_payload != nil {
+		pointerHandles.Untrack(ptr.progress_payload)
 	}
 }
 
@@ -185,11 +208,16 @@ func (v *Repository) CheckoutHead(opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_head(v.ptr, cOpts)
 	runtime.KeepAlive(v)
+
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
@@ -209,11 +237,15 @@ func (v *Repository) CheckoutIndex(index *Index, opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_index(v.ptr, iptr, cOpts)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
@@ -225,11 +257,16 @@ func (v *Repository) CheckoutTree(tree *Tree, opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_tree(v.ptr, tree.ptr, cOpts)
 	runtime.KeepAlive(v)
+	runtime.KeepAlive(tree)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}

cherrypick.go

@@ -25,7 +25,7 @@ func cherrypickOptionsFromC(c *C.git_cherrypick_options) CherrypickOptions {
 	return opts
 }
 
-func (opts *CherrypickOptions) toC() *C.git_cherrypick_options {
+func (opts *CherrypickOptions) toC(errorTarget *error) *C.git_cherrypick_options {
 	if opts == nil {
 		return nil
 	}
@@ -33,7 +33,7 @@ func (opts *CherrypickOptions) toC() *C.git_cherrypick_options {
 	c.version = C.uint(opts.Version)
 	c.mainline = C.uint(opts.Mainline)
 	c.merge_opts = *opts.MergeOpts.toC()
-	c.checkout_opts = *opts.CheckoutOpts.toC()
+	c.checkout_opts = *opts.CheckoutOpts.toC(errorTarget)
 	return &c
 }
 
@@ -41,6 +41,7 @@ func freeCherrypickOpts(ptr *C.git_cherrypick_options) {
 	if ptr == nil {
 		return
 	}
+	freeMergeOptions(&ptr.merge_opts)
 	freeCheckoutOptions(&ptr.checkout_opts)
 }
 
@@ -62,14 +63,18 @@ func (v *Repository) Cherrypick(commit *Commit, opts CherrypickOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCherrypickOpts(cOpts)
 
-	ecode := C.git_cherrypick(v.ptr, commit.cast_ptr, cOpts)
+	ret := C.git_cherrypick(v.ptr, commit.cast_ptr, cOpts)
 	runtime.KeepAlive(v)
 	runtime.KeepAlive(commit)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(ecode)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 	return nil
 }
@@ -79,6 +84,7 @@ func (r *Repository) CherrypickCommit(pick, our *Commit, opts CherrypickOptions)
 	defer runtime.UnlockOSThread()
 
 	cOpts := opts.MergeOpts.toC()
+	defer freeMergeOptions(cOpts)
 
 	var ptr *C.git_index
 	ret := C.git_cherrypick_commit(&ptr, r.ptr, pick.cast_ptr, our.cast_ptr, C.uint(opts.Mainline), cOpts)

clone.go

@@ -3,10 +3,11 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_populate_remote_cb(git_clone_options *opts);
+extern void _go_git_populate_clone_callbacks(git_clone_options *opts);
 */
 import "C"
 import (
+	"errors"
 	"runtime"
 	"unsafe"
 )
@@ -28,20 +29,23 @@ func Clone(url string, path string, options *CloneOptions) (*Repository, error)
 	cpath := C.CString(path)
 	defer C.free(unsafe.Pointer(cpath))
 
-	copts := (*C.git_clone_options)(C.calloc(1, C.size_t(unsafe.Sizeof(C.git_clone_options{}))))
+	var err error
-	populateCloneOptions(copts, options)
+	cOptions := populateCloneOptions(&C.git_clone_options{}, options, &err)
-	defer freeCloneOptions(copts)
+	defer freeCloneOptions(cOptions)
 
 	if len(options.CheckoutBranch) != 0 {
-		copts.checkout_branch = C.CString(options.CheckoutBranch)
+		cOptions.checkout_branch = C.CString(options.CheckoutBranch)
 	}
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var ptr *C.git_repository
-	ret := C.git_clone(&ptr, curl, cpath, copts)
+	ret := C.git_clone(&ptr, curl, cpath, cOptions)
 
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return nil, err
+	}
 	if ret < 0 {
 		return nil, MakeGitError(ret)
 	}
@@ -50,47 +54,69 @@ func Clone(url string, path string, options *CloneOptions) (*Repository, error)
 }
 
 //export remoteCreateCallback
-func remoteCreateCallback(cremote unsafe.Pointer, crepo unsafe.Pointer, cname, curl *C.char, payload unsafe.Pointer) C.int {
+func remoteCreateCallback(
+	cremote unsafe.Pointer,
+	crepo unsafe.Pointer,
+	cname, curl *C.char,
+	payload unsafe.Pointer,
+) C.int {
 	name := C.GoString(cname)
 	url := C.GoString(curl)
 	repo := newRepositoryFromC((*C.git_repository)(crepo))
 	// We don't own this repository, so make sure we don't try to free it
 	runtime.SetFinalizer(repo, nil)
 
-	if opts, ok := pointerHandles.Get(payload).(CloneOptions); ok {
+	data, ok := pointerHandles.Get(payload).(*cloneCallbackData)
-		remote, errorCode := opts.RemoteCreateCallback(repo, name, url)
+	if !ok {
-		// clear finalizer as the calling C function will
-		// free the remote itself
-		runtime.SetFinalizer(remote, nil)
-
-		if errorCode == ErrorCodeOK && remote != nil {
-			cptr := (**C.git_remote)(cremote)
-			*cptr = remote.ptr
-		} else if errorCode == ErrorCodeOK && remote == nil {
-			panic("no remote created by callback")
-		}
-
-		return C.int(errorCode)
-	} else {
 		panic("invalid remote create callback")
 	}
+
+	remote, ret := data.options.RemoteCreateCallback(repo, name, url)
+	// clear finalizer as the calling C function will
+	// free the remote itself
+	runtime.SetFinalizer(remote, nil)
+
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+
+	if remote == nil {
+		panic("no remote created by callback")
+	}
+
+	cptr := (**C.git_remote)(cremote)
+	*cptr = remote.ptr
+
+	return C.int(ErrorCodeOK)
 }
 
-func populateCloneOptions(ptr *C.git_clone_options, opts *CloneOptions) {
+type cloneCallbackData struct {
+	options     *CloneOptions
+	errorTarget *error
+}
+
+func populateCloneOptions(ptr *C.git_clone_options, opts *CloneOptions, errorTarget *error) *C.git_clone_options {
 	C.git_clone_options_init(ptr, C.GIT_CLONE_OPTIONS_VERSION)
 
 	if opts == nil {
-		return
+		return nil
 	}
-	populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts)
+	populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts, errorTarget)
 	populateFetchOptions(&ptr.fetch_opts, opts.FetchOptions)
 	ptr.bare = cbool(opts.Bare)
 
 	if opts.RemoteCreateCallback != nil {
+		data := &cloneCallbackData{
+			options:     opts,
+			errorTarget: errorTarget,
+		}
 		// Go v1.1 does not allow to assign a C function pointer
-		C._go_git_populate_remote_cb(ptr)
+		C._go_git_populate_clone_callbacks(ptr)
-		ptr.remote_cb_payload = pointerHandles.Track(*opts)
+		ptr.remote_cb_payload = pointerHandles.Track(data)
 	}
+
+	return ptr
 }
 
 func freeCloneOptions(ptr *C.git_clone_options) {
@@ -105,5 +131,4 @@ func freeCloneOptions(ptr *C.git_clone_options) {
 	}
 
 	C.free(unsafe.Pointer(ptr.checkout_branch))
-	C.free(unsafe.Pointer(ptr))
 }

diff.go

@@ -3,7 +3,7 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_populate_apply_cb(git_apply_options *options);
+extern void _go_git_populate_apply_callbacks(git_apply_options *options);
 extern int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLine, void *payload);
 extern void _go_git_setup_diff_notify_callbacks(git_diff_options* opts);
 extern int _go_git_diff_blobs(git_blob *old, const char *old_path, git_blob *new, const char *new_path, git_diff_options *opts, int eachFile, int eachHunk, int eachLine, void *payload);
@@ -294,11 +294,11 @@ func (diff *Diff) Stats() (*DiffStats, error) {
 	return stats, nil
 }
 
-type diffForEachData struct {
+type diffForEachCallbackData struct {
-	FileCallback DiffForEachFileCallback
+	fileCallback DiffForEachFileCallback
-	HunkCallback DiffForEachHunkCallback
+	hunkCallback DiffForEachHunkCallback
-	LineCallback DiffForEachLineCallback
+	lineCallback DiffForEachLineCallback
-	Error        error
+	errorTarget  *error
 }
 
 type DiffForEachFileCallback func(delta DiffDelta, progress float64) (DiffForEachHunkCallback, error)
@@ -326,82 +326,91 @@ func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) err
 		intLines = C.int(1)
 	}
 
-	data := &diffForEachData{
+	var err error
-		FileCallback: cbFile,
+	data := &diffForEachCallbackData{
+		fileCallback: cbFile,
+		errorTarget:  &err,
 	}
 
 	handle := pointerHandles.Track(data)
 	defer pointerHandles.Untrack(handle)
 
-	ecode := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, handle)
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	ret := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, handle)
 	runtime.KeepAlive(diff)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return data.Error
+		return err
 	}
+	if ret < 0 {
+		return MakeGitError(ret)
+	}
+
 	return nil
 }
 
-//export diffForEachFileCb
+//export diffForEachFileCallback
-func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, handle unsafe.Pointer) int {
+func diffForEachFileCallback(delta *C.git_diff_delta, progress C.float, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*diffForEachData)
+	data, ok := payload.(*diffForEachCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
 
-	data.HunkCallback = nil
+	data.hunkCallback = nil
-	if data.FileCallback != nil {
+	if data.fileCallback != nil {
-		cb, err := data.FileCallback(diffDeltaFromC(delta), float64(progress))
+		cb, err := data.fileCallback(diffDeltaFromC(delta), float64(progress))
 		if err != nil {
-			data.Error = err
+			*data.errorTarget = err
-			return -1
+			return C.int(ErrorCodeUser)
 		}
-		data.HunkCallback = cb
+		data.hunkCallback = cb
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 type DiffForEachHunkCallback func(DiffHunk) (DiffForEachLineCallback, error)
 
-//export diffForEachHunkCb
+//export diffForEachHunkCallback
-func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, handle unsafe.Pointer) int {
+func diffForEachHunkCallback(delta *C.git_diff_delta, hunk *C.git_diff_hunk, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*diffForEachData)
+	data, ok := payload.(*diffForEachCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
 
-	data.LineCallback = nil
+	data.lineCallback = nil
-	if data.HunkCallback != nil {
+	if data.hunkCallback != nil {
-		cb, err := data.HunkCallback(diffHunkFromC(hunk))
+		cb, err := data.hunkCallback(diffHunkFromC(hunk))
 		if err != nil {
-			data.Error = err
+			*data.errorTarget = err
-			return -1
+			return C.int(ErrorCodeUser)
 		}
-		data.LineCallback = cb
+		data.lineCallback = cb
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 type DiffForEachLineCallback func(DiffLine) error
 
-//export diffForEachLineCb
+//export diffForEachLineCallback
-func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, handle unsafe.Pointer) int {
+func diffForEachLineCallback(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*diffForEachData)
+	data, ok := payload.(*diffForEachCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
 
-	err := data.LineCallback(diffLineFromC(line))
+	err := data.lineCallback(diffLineFromC(line))
 	if err != nil {
-		data.Error = err
+		*data.errorTarget = err
-		return -1
+		return C.int(ErrorCodeUser)
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 func (diff *Diff) Patch(deltaIndex int) (*Patch, error) {
@@ -586,93 +595,98 @@ var (
 	ErrDeltaSkip = errors.New("Skip delta")
 )
 
-type diffNotifyData struct {
+type diffNotifyCallbackData struct {
-	Callback   DiffNotifyCallback
+	callback    DiffNotifyCallback
-	Repository *Repository
+	repository  *Repository
-	Error      error
+	errorTarget *error
 }
 
-//export diffNotifyCb
+//export diffNotifyCallback
-func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, handle unsafe.Pointer) int {
+func diffNotifyCallback(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, handle unsafe.Pointer) C.int {
 	diff_so_far := (*C.git_diff)(_diff_so_far)
 
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*diffNotifyData)
+	data, ok := payload.(*diffNotifyCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
 
-	if data != nil {
+	if data == nil {
-		// We are not taking ownership of this diff pointer, so no finalizer is set.
+		return C.int(ErrorCodeOK)
-		diff := &Diff{
-			ptr:          diff_so_far,
-			repo:         data.Repository,
-			runFinalizer: false,
-		}
-
-		err := data.Callback(diff, diffDeltaFromC(delta_to_add), C.GoString(matched_pathspec))
-
-		// Since the callback could theoretically keep a reference to the diff
-		// (which could be freed by libgit2 if an error occurs later during the
-		// diffing process), this converts a use-after-free (terrible!) into a nil
-		// dereference ("just" pretty bad).
-		diff.ptr = nil
-
-		if err == ErrDeltaSkip {
-			return 1
-		} else if err != nil {
-			data.Error = err
-			return -1
-		} else {
-			return 0
-		}
 	}
-	return 0
+
+	// We are not taking ownership of this diff pointer, so no finalizer is set.
+	diff := &Diff{
+		ptr:          diff_so_far,
+		repo:         data.repository,
+		runFinalizer: false,
+	}
+
+	err := data.callback(diff, diffDeltaFromC(delta_to_add), C.GoString(matched_pathspec))
+
+	// Since the callback could theoretically keep a reference to the diff
+	// (which could be freed by libgit2 if an error occurs later during the
+	// diffing process), this converts a use-after-free (terrible!) into a nil
+	// dereference ("just" pretty bad).
+	diff.ptr = nil
+
+	if err == ErrDeltaSkip {
+		return 1
+	}
+	if err != nil {
+		*data.errorTarget = err
+		return C.int(ErrorCodeUser)
+	}
+
+	return C.int(ErrorCodeOK)
 }
 
-func diffOptionsToC(opts *DiffOptions, repo *Repository) (copts *C.git_diff_options) {
+func (opts *DiffOptions) toC(repo *Repository, errorTarget *error) *C.git_diff_options {
-	cpathspec := C.git_strarray{}
+	if opts == nil {
-	if opts != nil {
+		return nil
-		notifyData := &diffNotifyData{
-			Callback:   opts.NotifyCallback,
-			Repository: repo,
-		}
-
-		if opts.Pathspec != nil {
-			cpathspec.count = C.size_t(len(opts.Pathspec))
-			cpathspec.strings = makeCStringsFromStrings(opts.Pathspec)
-		}
-
-		copts = &C.git_diff_options{
-			version:           C.GIT_DIFF_OPTIONS_VERSION,
-			flags:             C.uint32_t(opts.Flags),
-			ignore_submodules: C.git_submodule_ignore_t(opts.IgnoreSubmodules),
-			pathspec:          cpathspec,
-			context_lines:     C.uint32_t(opts.ContextLines),
-			interhunk_lines:   C.uint32_t(opts.InterhunkLines),
-			id_abbrev:         C.uint16_t(opts.IdAbbrev),
-			max_size:          C.git_off_t(opts.MaxSize),
-			old_prefix:        C.CString(opts.OldPrefix),
-			new_prefix:        C.CString(opts.NewPrefix),
-		}
-
-		if opts.NotifyCallback != nil {
-			C._go_git_setup_diff_notify_callbacks(copts)
-			copts.payload = pointerHandles.Track(notifyData)
-		}
 	}
-	return
+
+	cpathspec := C.git_strarray{}
+	if opts.Pathspec != nil {
+		cpathspec.count = C.size_t(len(opts.Pathspec))
+		cpathspec.strings = makeCStringsFromStrings(opts.Pathspec)
+	}
+
+	copts := &C.git_diff_options{
+		version:           C.GIT_DIFF_OPTIONS_VERSION,
+		flags:             C.uint32_t(opts.Flags),
+		ignore_submodules: C.git_submodule_ignore_t(opts.IgnoreSubmodules),
+		pathspec:          cpathspec,
+		context_lines:     C.uint32_t(opts.ContextLines),
+		interhunk_lines:   C.uint32_t(opts.InterhunkLines),
+		id_abbrev:         C.uint16_t(opts.IdAbbrev),
+		max_size:          C.git_off_t(opts.MaxSize),
+		old_prefix:        C.CString(opts.OldPrefix),
+		new_prefix:        C.CString(opts.NewPrefix),
+	}
+
+	if opts.NotifyCallback != nil {
+		notifyData := &diffNotifyCallbackData{
+			callback:    opts.NotifyCallback,
+			repository:  repo,
+			errorTarget: errorTarget,
+		}
+		C._go_git_setup_diff_notify_callbacks(copts)
+		copts.payload = pointerHandles.Track(notifyData)
+	}
+	return copts
 }
 
 func freeDiffOptions(copts *C.git_diff_options) {
-	if copts != nil {
+	if copts == nil {
-		cpathspec := copts.pathspec
+		return
-		freeStrarray(&cpathspec)
+	}
-		C.free(unsafe.Pointer(copts.old_prefix))
+	cpathspec := copts.pathspec
-		C.free(unsafe.Pointer(copts.new_prefix))
+	freeStrarray(&cpathspec)
-		if copts.payload != nil {
+	C.free(unsafe.Pointer(copts.old_prefix))
-			pointerHandles.Untrack(copts.payload)
+	C.free(unsafe.Pointer(copts.new_prefix))
-		}
+	if copts.payload != nil {
+		pointerHandles.Untrack(copts.payload)
 	}
 }
 
@@ -688,17 +702,21 @@ func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree, opts *DiffOptions) (
 		newPtr = newTree.cast_ptr
 	}
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, copts)
+	ret := C.git_diff_tree_to_tree(&diffPtr, v.ptr, oldPtr, newPtr, copts)
 	runtime.KeepAlive(oldTree)
 	runtime.KeepAlive(newTree)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
+	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
 	}
 	return newDiffFromC(diffPtr, v), nil
 }
@@ -711,17 +729,22 @@ func (v *Repository) DiffTreeToWorkdir(oldTree *Tree, opts *DiffOptions) (*Diff,
 		oldPtr = oldTree.cast_ptr
 	}
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_diff_tree_to_workdir(&diffPtr, v.ptr, oldPtr, copts)
+	ret := C.git_diff_tree_to_workdir(&diffPtr, v.ptr, oldPtr, copts)
 	runtime.KeepAlive(oldTree)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
 	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
 	return newDiffFromC(diffPtr, v), nil
 }
 
@@ -738,18 +761,23 @@ func (v *Repository) DiffTreeToIndex(oldTree *Tree, index *Index, opts *DiffOpti
 		indexPtr = index.ptr
 	}
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_diff_tree_to_index(&diffPtr, v.ptr, oldPtr, indexPtr, copts)
+	ret := C.git_diff_tree_to_index(&diffPtr, v.ptr, oldPtr, indexPtr, copts)
 	runtime.KeepAlive(oldTree)
 	runtime.KeepAlive(index)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
 	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
 	return newDiffFromC(diffPtr, v), nil
 }
 
@@ -761,17 +789,22 @@ func (v *Repository) DiffTreeToWorkdirWithIndex(oldTree *Tree, opts *DiffOptions
 		oldPtr = oldTree.cast_ptr
 	}
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_diff_tree_to_workdir_with_index(&diffPtr, v.ptr, oldPtr, copts)
+	ret := C.git_diff_tree_to_workdir_with_index(&diffPtr, v.ptr, oldPtr, copts)
 	runtime.KeepAlive(oldTree)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
 	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
 	return newDiffFromC(diffPtr, v), nil
 }
 
@@ -783,25 +816,32 @@ func (v *Repository) DiffIndexToWorkdir(index *Index, opts *DiffOptions) (*Diff,
 		indexPtr = index.ptr
 	}
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_diff_index_to_workdir(&diffPtr, v.ptr, indexPtr, copts)
+	ret := C.git_diff_index_to_workdir(&diffPtr, v.ptr, indexPtr, copts)
 	runtime.KeepAlive(index)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
 	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
 	return newDiffFromC(diffPtr, v), nil
 }
 
 // DiffBlobs performs a diff between two arbitrary blobs. You can pass
 // whatever file names you'd like for them to appear as in the diff.
 func DiffBlobs(oldBlob *Blob, oldAsPath string, newBlob *Blob, newAsPath string, opts *DiffOptions, fileCallback DiffForEachFileCallback, detail DiffDetail) error {
-	data := &diffForEachData{
+	var err error
-		FileCallback: fileCallback,
+	data := &diffForEachCallbackData{
+		fileCallback: fileCallback,
+		errorTarget:  &err,
 	}
 
 	intHunks := C.int(0)
@@ -833,17 +873,20 @@ func DiffBlobs(oldBlob *Blob, oldAsPath string, newBlob *Blob, newAsPath string,
 	newBlobPath := C.CString(newAsPath)
 	defer C.free(unsafe.Pointer(newBlobPath))
 
-	copts := diffOptionsToC(opts, repo)
+	copts := opts.toC(repo, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C._go_git_diff_blobs(oldBlobPtr, oldBlobPath, newBlobPtr, newBlobPath, copts, 1, intHunks, intLines, handle)
+	ret := C._go_git_diff_blobs(oldBlobPtr, oldBlobPath, newBlobPtr, newBlobPath, copts, 1, intHunks, intLines, handle)
 	runtime.KeepAlive(oldBlob)
 	runtime.KeepAlive(newBlob)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(ecode)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil
@@ -864,56 +907,59 @@ type ApplyOptions struct {
 	Flags              uint
 }
 
+type applyCallbackData struct {
+	options     *ApplyOptions
+	errorTarget *error
+}
+
 //export hunkApplyCallback
 func hunkApplyCallback(_hunk *C.git_diff_hunk, _payload unsafe.Pointer) C.int {
-	opts, ok := pointerHandles.Get(_payload).(*ApplyOptions)
+	data, ok := pointerHandles.Get(_payload).(*applyCallbackData)
 	if !ok {
 		panic("invalid apply options payload")
 	}
 
-	if opts.ApplyHunkCallback == nil {
+	if data.options.ApplyHunkCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 
 	hunk := diffHunkFromC(_hunk)
 
-	apply, err := opts.ApplyHunkCallback(&hunk)
+	apply, err := data.options.ApplyHunkCallback(&hunk)
 	if err != nil {
-		if gitError, ok := err.(*GitError); ok {
+		*data.errorTarget = err
-			return C.int(gitError.Code)
+		return C.int(ErrorCodeUser)
-		}
+	}
-		return -1
+
-	} else if apply {
+	if !apply {
-		return 0
-	} else {
 		return 1
 	}
+	return C.int(ErrorCodeOK)
 }
 
 //export deltaApplyCallback
 func deltaApplyCallback(_delta *C.git_diff_delta, _payload unsafe.Pointer) C.int {
-	opts, ok := pointerHandles.Get(_payload).(*ApplyOptions)
+	data, ok := pointerHandles.Get(_payload).(*applyCallbackData)
 	if !ok {
 		panic("invalid apply options payload")
 	}
 
-	if opts.ApplyDeltaCallback == nil {
+	if data.options.ApplyDeltaCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 
 	delta := diffDeltaFromC(_delta)
 
-	apply, err := opts.ApplyDeltaCallback(&delta)
+	apply, err := data.options.ApplyDeltaCallback(&delta)
 	if err != nil {
-		if gitError, ok := err.(*GitError); ok {
+		*data.errorTarget = err
-			return C.int(gitError.Code)
+		return C.int(ErrorCodeUser)
-		}
+	}
-		return -1
+
-	} else if apply {
+	if !apply {
-		return 0
-	} else {
 		return 1
 	}
+	return C.int(ErrorCodeOK)
 }
 
 // DefaultApplyOptions returns default options for applying diffs or patches.
@@ -925,14 +971,13 @@ func DefaultApplyOptions() (*ApplyOptions, error) {
 
 	ecode := C.git_apply_options_init(&opts, C.GIT_APPLY_OPTIONS_VERSION)
 	if int(ecode) != 0 {
-
 		return nil, MakeGitError(ecode)
 	}
 
 	return applyOptionsFromC(&opts), nil
 }
 
-func (a *ApplyOptions) toC() *C.git_apply_options {
+func (a *ApplyOptions) toC(errorTarget *error) *C.git_apply_options {
 	if a == nil {
 		return nil
 	}
@@ -943,13 +988,26 @@ func (a *ApplyOptions) toC() *C.git_apply_options {
 	}
 
 	if a.ApplyDeltaCallback != nil || a.ApplyHunkCallback != nil {
-		C._go_git_populate_apply_cb(opts)
+		data := &applyCallbackData{
-		opts.payload = pointerHandles.Track(a)
+			options:     a,
+			errorTarget: errorTarget,
+		}
+		C._go_git_populate_apply_callbacks(opts)
+		opts.payload = pointerHandles.Track(data)
 	}
 
 	return opts
 }
 
+func freeApplyOptions(opts *C.git_apply_options) {
+	if opts == nil {
+		return
+	}
+	if opts.payload != nil {
+		pointerHandles.Untrack(opts.payload)
+	}
+}
+
 func applyOptionsFromC(opts *C.git_apply_options) *ApplyOptions {
 	return &ApplyOptions{
 		Flags: uint(opts.flags),
@@ -979,13 +1037,19 @@ func (v *Repository) ApplyDiff(diff *Diff, location ApplyLocation, opts *ApplyOp
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
-	ecode := C.git_apply(v.ptr, diff.ptr, C.git_apply_location_t(location), cOpts)
+	cOpts := opts.toC(&err)
+	defer freeApplyOptions(cOpts)
+
+	ret := C.git_apply(v.ptr, diff.ptr, C.git_apply_location_t(location), cOpts)
 	runtime.KeepAlive(v)
 	runtime.KeepAlive(diff)
 	runtime.KeepAlive(cOpts)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(ecode)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil
@@ -996,14 +1060,20 @@ func (v *Repository) ApplyToTree(diff *Diff, tree *Tree, opts *ApplyOptions) (*I
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
+	var err error
+	cOpts := opts.toC(&err)
+	defer freeApplyOptions(cOpts)
+
 	var indexPtr *C.git_index
-	cOpts := opts.toC()
+	ret := C.git_apply_to_tree(&indexPtr, v.ptr, tree.cast_ptr, diff.ptr, cOpts)
-	ecode := C.git_apply_to_tree(&indexPtr, v.ptr, tree.cast_ptr, diff.ptr, cOpts)
 	runtime.KeepAlive(diff)
 	runtime.KeepAlive(tree)
 	runtime.KeepAlive(cOpts)
-	if ecode != 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
+	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
 	}
 
 	return newIndexFromC(indexPtr, v), nil

diff_test.go

@@ -173,9 +173,8 @@ func TestDiffTreeToTree(t *testing.T) {
 	}, DiffDetailLines)
 
 	if err != errTest {
-		t.Fatal("Expected custom error to be returned")
+		t.Fatalf("Expected custom error to be returned, got %v, want %v", err, errTest)
 	}
-
 }
 
 func createTestTrees(t *testing.T, repo *Repository) (originalTree *Tree, newTree *Tree) {
@@ -486,13 +485,15 @@ func TestApplyToTree(t *testing.T) {
 	diffAC, err := repo.DiffTreeToTree(treeA, treeC, nil)
 	checkFatal(t, err)
 
+	errMessageDropped := errors.New("message dropped")
+
 	for _, tc := range []struct {
 		name               string
 		tree               *Tree
 		diff               *Diff
 		applyHunkCallback  ApplyHunkCallback
 		applyDeltaCallback ApplyDeltaCallback
-		error              error
+		err                error
 		expectedDiff       *Diff
 	}{
 		{
@@ -505,7 +506,7 @@ func TestApplyToTree(t *testing.T) {
 			name: "applying a conflicting patch errors",
 			tree: treeB,
 			diff: diffAC,
-			error: &GitError{
+			err: &GitError{
 				Message: "hunk at line 1 did not apply",
 				Code:    ErrorCodeApplyFail,
 				Class:   ErrorClassPatch,
@@ -529,12 +530,8 @@ func TestApplyToTree(t *testing.T) {
 			name:              "hunk callback erroring fails the call",
 			tree:              treeA,
 			diff:              diffAB,
-			applyHunkCallback: func(*DiffHunk) (bool, error) { return true, errors.New("message dropped") },
+			applyHunkCallback: func(*DiffHunk) (bool, error) { return true, errMessageDropped },
-			error: &GitError{
+			err:               errMessageDropped,
-				Message: "Generic",
-				Code:    ErrorCodeGeneric,
-				Class:   ErrorClassInvalid,
-			},
 		},
 		{
 			name:               "delta callback returning false does not apply",
@@ -546,12 +543,8 @@ func TestApplyToTree(t *testing.T) {
 			name:               "delta callback erroring fails the call",
 			tree:               treeA,
 			diff:               diffAB,
-			applyDeltaCallback: func(*DiffDelta) (bool, error) { return true, errors.New("message dropped") },
+			applyDeltaCallback: func(*DiffDelta) (bool, error) { return true, errMessageDropped },
-			error: &GitError{
+			err:                errMessageDropped,
-				Message: "Generic",
-				Code:    ErrorCodeGeneric,
-				Class:   ErrorClassInvalid,
-			},
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -562,9 +555,9 @@ func TestApplyToTree(t *testing.T) {
 			opts.ApplyDeltaCallback = tc.applyDeltaCallback
 
 			index, err := repo.ApplyToTree(tc.diff, tc.tree, opts)
-			if tc.error != nil {
+			if tc.err != nil {
-				if !reflect.DeepEqual(err, tc.error) {
+				if !reflect.DeepEqual(tc.err, err) {
-					t.Fatalf("expected error %q but got %q", tc.error, err)
+					t.Fatalf("expected error %q but got %q", tc.err, err)
 				}
 
 				return

git.go

@@ -330,6 +330,17 @@ func ucbool(b bool) C.uint {
 	return C.uint(0)
 }
 
+func setCallbackError(errorMessage **C.char, err error) C.int {
+	if err != nil {
+		*errorMessage = C.CString(err.Error())
+		if gitError, ok := err.(*GitError); ok {
+			return C.int(gitError.Code)
+		}
+		return C.int(ErrorCodeUser)
+	}
+	return C.int(ErrorCodeOK)
+}
+
 func Discover(start string, across_fs bool, ceiling_dirs []string) (string, error) {
 	ceildirs := C.CString(strings.Join(ceiling_dirs, string(C.GIT_PATH_LIST_SEPARATOR)))
 	defer C.free(unsafe.Pointer(ceildirs))

index.go

@@ -10,12 +10,17 @@ extern int _go_git_index_remove_all(git_index*, const git_strarray*, void*);
 */
 import "C"
 import (
+	"errors"
 	"fmt"
 	"runtime"
 	"unsafe"
 )
 
 type IndexMatchedPathCallback func(string, string) int
+type indexMatchedPathCallbackData struct {
+	callback    IndexMatchedPathCallback
+	errorTarget *error
+}
 
 // IndexAddOption is a set of flags for APIs that add files matching pathspec.
 type IndexAddOption uint
@@ -227,12 +232,17 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOption, callback IndexM
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -243,9 +253,13 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOption, callback IndexM
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
@@ -255,12 +269,17 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback)
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -270,9 +289,13 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback)
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
@@ -282,12 +305,17 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback)
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -297,19 +325,30 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback)
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
 //export indexMatchedPathCallback
-func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int {
+func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) C.int {
-	if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok {
+	data, ok := pointerHandles.Get(payload).(*indexMatchedPathCallbackData)
-		return callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
+	if !ok {
-	} else {
 		panic("invalid matched path callback")
 	}
+
+	ret := data.callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+
+	return C.int(ErrorCodeOK)
 }
 
 func (v *Index) RemoveByPath(path string) error {
@@ -435,7 +474,7 @@ func (v *Index) EntryByPath(path string, stage int) (*IndexEntry, error) {
 
 	centry := C.git_index_get_bypath(v.ptr, cpath, C.int(stage))
 	if centry == nil {
-		return nil, MakeGitError(C.GIT_ENOTFOUND)
+		return nil, MakeGitError(C.int(ErrorCodeNotFound))
 	}
 	ret := newIndexEntryFromC(centry)
 	runtime.KeepAlive(v)

merge.go

@@ -186,6 +186,9 @@ func (mo *MergeOptions) toC() *C.git_merge_options {
 	}
 }
 
+func freeMergeOptions(opts *C.git_merge_options) {
+}
+
 type MergeFileFavor int
 
 const (
@@ -199,8 +202,10 @@ func (r *Repository) Merge(theirHeads []*AnnotatedCommit, mergeOptions *MergeOpt
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
+	var err error
 	cMergeOpts := mergeOptions.toC()
-	cCheckoutOptions := checkoutOptions.toC()
+	defer freeMergeOptions(cMergeOpts)
+	cCheckoutOptions := checkoutOptions.toC(&err)
 	defer freeCheckoutOptions(cCheckoutOptions)
 
 	gmerge_head_array := make([]*C.git_annotated_commit, len(theirHeads))
@@ -208,10 +213,13 @@ func (r *Repository) Merge(theirHeads []*AnnotatedCommit, mergeOptions *MergeOpt
 		gmerge_head_array[i] = theirHeads[i].ptr
 	}
 	ptr := unsafe.Pointer(&gmerge_head_array[0])
-	err := C.git_merge(r.ptr, (**C.git_annotated_commit)(ptr), C.size_t(len(theirHeads)), cMergeOpts, cCheckoutOptions)
+	ret := C.git_merge(r.ptr, (**C.git_annotated_commit)(ptr), C.size_t(len(theirHeads)), cMergeOpts, cCheckoutOptions)
 	runtime.KeepAlive(theirHeads)
-	if err < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(err)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 	return nil
 }
@@ -262,6 +270,7 @@ func (r *Repository) MergeCommits(ours *Commit, theirs *Commit, options *MergeOp
 	defer runtime.UnlockOSThread()
 
 	copts := options.toC()
+	defer freeMergeOptions(copts)
 
 	var ptr *C.git_index
 	ret := C.git_merge_commits(&ptr, r.ptr, ours.cast_ptr, theirs.cast_ptr, copts)
@@ -279,6 +288,7 @@ func (r *Repository) MergeTrees(ancestor *Tree, ours *Tree, theirs *Tree, option
 	defer runtime.UnlockOSThread()
 
 	copts := options.toC()
+	defer freeMergeOptions(copts)
 
 	var ancestor_ptr *C.git_tree
 	if ancestor != nil {
@@ -446,6 +456,9 @@ func populateCMergeFileOptions(c *C.git_merge_file_options, options MergeFileOpt
 }
 
 func freeCMergeFileOptions(c *C.git_merge_file_options) {
+	if c == nil {
+		return
+	}
 	C.free(unsafe.Pointer(c.ancestor_label))
 	C.free(unsafe.Pointer(c.our_label))
 	C.free(unsafe.Pointer(c.their_label))

odb.go

@@ -175,35 +175,33 @@ func (v *Odb) Read(oid *Oid) (obj *OdbObject, err error) {
 }
 
 type OdbForEachCallback func(id *Oid) error
-
+type odbForEachCallbackData struct {
-type foreachData struct {
+	callback    OdbForEachCallback
-	callback OdbForEachCallback
+	errorTarget *error
-	err      error
 }
 
-//export odbForEachCb
+//export odbForEachCallback
-func odbForEachCb(id *C.git_oid, handle unsafe.Pointer) int {
+func odbForEachCallback(id *C.git_oid, handle unsafe.Pointer) C.int {
-	data, ok := pointerHandles.Get(handle).(*foreachData)
+	data, ok := pointerHandles.Get(handle).(*odbForEachCallbackData)
-
 	if !ok {
 		panic("could not retrieve handle")
 	}
 
 	err := data.callback(newOidFromC(id))
 	if err != nil {
-		data.err = err
+		*data.errorTarget = err
-		return C.GIT_EUSER
+		return C.int(ErrorCodeUser)
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 func (v *Odb) ForEach(callback OdbForEachCallback) error {
-	data := foreachData{
+	var err error
-		callback: callback,
+	data := odbForEachCallbackData{
-		err:      nil,
+		callback:    callback,
+		errorTarget: &err,
 	}
-
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
@@ -212,9 +210,10 @@ func (v *Odb) ForEach(callback OdbForEachCallback) error {
 
 	ret := C._go_git_odb_foreach(v.ptr, handle)
 	runtime.KeepAlive(v)
-	if ret == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return data.err
+		return err
-	} else if ret < 0 {
+	}
+	if ret < 0 {
 		return MakeGitError(ret)
 	}
 

packbuilder.go

@@ -133,15 +133,15 @@ func (pb *Packbuilder) Written() uint32 {
 }
 
 type PackbuilderForeachCallback func([]byte) error
-type packbuilderCbData struct {
+type packbuilderCallbackData struct {
-	callback PackbuilderForeachCallback
+	callback    PackbuilderForeachCallback
-	err      error
+	errorTarget *error
 }
 
-//export packbuilderForEachCb
+//export packbuilderForEachCallback
-func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) int {
+func packbuilderForEachCallback(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*packbuilderCbData)
+	data, ok := payload.(*packbuilderCallbackData)
 	if !ok {
 		panic("could not get packbuilder CB data")
 	}
@@ -150,19 +150,20 @@ func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Point
 
 	err := data.callback(slice)
 	if err != nil {
-		data.err = err
+		*data.errorTarget = err
-		return C.GIT_EUSER
+		return C.int(ErrorCodeUser)
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 // ForEach repeatedly calls the callback with new packfile data until
 // there is no more data or the callback returns an error
 func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error {
-	data := packbuilderCbData{
+	var err error
-		callback: callback,
+	data := packbuilderCallbackData{
-		err:      nil,
+		callback:    callback,
+		errorTarget: &err,
 	}
 	handle := pointerHandles.Track(&data)
 	defer pointerHandles.Untrack(handle)
@@ -170,13 +171,13 @@ func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	err := C._go_git_packbuilder_foreach(pb.ptr, handle)
+	ret := C._go_git_packbuilder_foreach(pb.ptr, handle)
 	runtime.KeepAlive(pb)
-	if err == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return data.err
+		return err
 	}
-	if err < 0 {
+	if ret < 0 {
-		return MakeGitError(err)
+		return MakeGitError(ret)
 	}
 
 	return nil

patch.go

@@ -77,17 +77,22 @@ func (v *Repository) PatchFromBuffers(oldPath, newPath string, oldBuf, newBuf []
 	cNewPath := C.CString(newPath)
 	defer C.free(unsafe.Pointer(cNewPath))
 
-	copts := diffOptionsToC(opts, v)
+	var err error
+	copts := opts.toC(v, &err)
 	defer freeDiffOptions(copts)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ecode := C.git_patch_from_buffers(&patchPtr, oldPtr, C.size_t(len(oldBuf)), cOldPath, newPtr, C.size_t(len(newBuf)), cNewPath, copts)
+	ret := C.git_patch_from_buffers(&patchPtr, oldPtr, C.size_t(len(oldBuf)), cOldPath, newPtr, C.size_t(len(newBuf)), cNewPath, copts)
 	runtime.KeepAlive(oldBuf)
 	runtime.KeepAlive(newBuf)
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(ecode)
+		return nil, err
 	}
+	if ret < 0 {
+		return nil, MakeGitError(ret)
+	}
+
 	return newPatchFromC(patchPtr), nil
 }

rebase.go

@@ -3,7 +3,7 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_populate_commit_sign_cb(git_rebase_options *opts);
+extern void _go_git_populate_rebase_callbacks(git_rebase_options *opts);
 */
 import "C"
 import (
@@ -71,25 +71,22 @@ func newRebaseOperationFromC(c *C.git_rebase_operation) *RebaseOperation {
 	return operation
 }
 
-//export commitSignCallback
+//export commitSigningCallback
-func commitSignCallback(_signature *C.git_buf, _signature_field *C.git_buf, _commit_content *C.char, _payload unsafe.Pointer) C.int {
+func commitSigningCallback(errorMessage **C.char, _signature *C.git_buf, _signature_field *C.git_buf, _commit_content *C.char, _payload unsafe.Pointer) C.int {
 	opts, ok := pointerHandles.Get(_payload).(*RebaseOptions)
 	if !ok {
 		panic("invalid sign payload")
 	}
 
 	if opts.CommitSigningCallback == nil {
-		return C.GIT_PASSTHROUGH
+		return C.int(ErrorCodePassthrough)
 	}
 
 	commitContent := C.GoString(_commit_content)
 
 	signature, signatureField, err := opts.CommitSigningCallback(commitContent)
 	if err != nil {
-		if gitError, ok := err.(*GitError); ok {
+		return setCallbackError(errorMessage, err)
-			return C.int(gitError.Code)
-		}
-		return C.int(-1)
 	}
 
 	fillBuf := func(bufData string, buf *C.git_buf) error {
@@ -110,16 +107,16 @@ func commitSignCallback(_signature *C.git_buf, _signature_field *C.git_buf, _com
 	if signatureField != "" {
 		err := fillBuf(signatureField, _signature_field)
 		if err != nil {
-			return C.int(-1)
+			return setCallbackError(errorMessage, err)
 		}
 	}
 
 	err = fillBuf(signature, _signature)
 	if err != nil {
-		return C.int(-1)
+		return setCallbackError(errorMessage, err)
 	}
 
-	return C.GIT_OK
+	return C.int(ErrorCodeOK)
 }
 
 // RebaseOptions are used to tell the rebase machinery how to operate
@@ -158,25 +155,38 @@ func rebaseOptionsFromC(opts *C.git_rebase_options) RebaseOptions {
 	}
 }
 
-func (ro *RebaseOptions) toC() *C.git_rebase_options {
+func (ro *RebaseOptions) toC(errorTarget *error) *C.git_rebase_options {
 	if ro == nil {
 		return nil
 	}
-	opts := &C.git_rebase_options{
+
+	cOptions := &C.git_rebase_options{
 		version:           C.uint(ro.Version),
 		quiet:             C.int(ro.Quiet),
 		inmemory:          C.int(ro.InMemory),
 		rewrite_notes_ref: mapEmptyStringToNull(ro.RewriteNotesRef),
 		merge_options:     *ro.MergeOptions.toC(),
-		checkout_options:  *ro.CheckoutOptions.toC(),
+		checkout_options:  *ro.CheckoutOptions.toC(errorTarget),
 	}
 
 	if ro.CommitSigningCallback != nil {
-		C._go_git_populate_commit_sign_cb(opts)
+		C._go_git_populate_rebase_callbacks(cOptions)
-		opts.payload = pointerHandles.Track(ro)
+		cOptions.payload = pointerHandles.Track(ro)
 	}
 
-	return opts
+	return cOptions
+}
+
+func freeRebaseOptions(opts *C.git_rebase_options) {
+	if opts == nil {
+		return
+	}
+	C.free(unsafe.Pointer(opts.rewrite_notes_ref))
+	freeMergeOptions(&opts.merge_options)
+	freeCheckoutOptions(&opts.checkout_options)
+	if opts.payload != nil {
+		pointerHandles.Untrack(opts.payload)
+	}
 }
 
 func mapEmptyStringToNull(ref string) *C.char {
@@ -188,8 +198,9 @@ func mapEmptyStringToNull(ref string) *C.char {
 
 // Rebase is the struct representing a Rebase object.
 type Rebase struct {
-	ptr *C.git_rebase
+	ptr     *C.git_rebase
-	r   *Repository
+	r       *Repository
+	options *C.git_rebase_options
 }
 
 // InitRebase initializes a rebase operation to rebase the changes in branch relative to upstream onto another branch.
@@ -210,15 +221,22 @@ func (r *Repository) InitRebase(branch *AnnotatedCommit, upstream *AnnotatedComm
 	}
 
 	var ptr *C.git_rebase
-	err := C.git_rebase_init(&ptr, r.ptr, branch.ptr, upstream.ptr, onto.ptr, opts.toC())
+	var err error
+	cOpts := opts.toC(&err)
+	ret := C.git_rebase_init(&ptr, r.ptr, branch.ptr, upstream.ptr, onto.ptr, cOpts)
 	runtime.KeepAlive(branch)
 	runtime.KeepAlive(upstream)
 	runtime.KeepAlive(onto)
-	if err < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(err)
+		freeRebaseOptions(cOpts)
+		return nil, err
+	}
+	if ret < 0 {
+		freeRebaseOptions(cOpts)
+		return nil, MakeGitError(ret)
 	}
 
-	return newRebaseFromC(ptr), nil
+	return newRebaseFromC(ptr, cOpts), nil
 }
 
 // OpenRebase opens an existing rebase that was previously started by either an invocation of InitRebase or by another client.
@@ -227,13 +245,20 @@ func (r *Repository) OpenRebase(opts *RebaseOptions) (*Rebase, error) {
 	defer runtime.UnlockOSThread()
 
 	var ptr *C.git_rebase
-	err := C.git_rebase_open(&ptr, r.ptr, opts.toC())
+	var err error
+	cOpts := opts.toC(&err)
+	ret := C.git_rebase_open(&ptr, r.ptr, cOpts)
 	runtime.KeepAlive(r)
-	if err < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return nil, MakeGitError(err)
+		freeRebaseOptions(cOpts)
+		return nil, err
+	}
+	if ret < 0 {
+		freeRebaseOptions(cOpts)
+		return nil, MakeGitError(ret)
 	}
 
-	return newRebaseFromC(ptr), nil
+	return newRebaseFromC(ptr, cOpts), nil
 }
 
 // OperationAt gets the rebase operation specified by the given index.
@@ -344,13 +369,14 @@ func (rebase *Rebase) Abort() error {
 }
 
 // Free frees the Rebase object.
-func (rebase *Rebase) Free() {
+func (r *Rebase) Free() {
-	runtime.SetFinalizer(rebase, nil)
+	runtime.SetFinalizer(r, nil)
-	C.git_rebase_free(rebase.ptr)
+	C.git_rebase_free(r.ptr)
+	freeRebaseOptions(r.options)
 }
 
-func newRebaseFromC(ptr *C.git_rebase) *Rebase {
+func newRebaseFromC(ptr *C.git_rebase, opts *C.git_rebase_options) *Rebase {
-	rebase := &Rebase{ptr: ptr}
+	rebase := &Rebase{ptr: ptr, options: opts}
 	runtime.SetFinalizer(rebase, (*Rebase).Free)
 	return rebase
 }

remote.go

@@ -6,12 +6,13 @@ package git
 #include <git2.h>
 #include <git2/sys/cred.h>
 
-extern void _go_git_setup_callbacks(git_remote_callbacks *callbacks);
+extern void _go_git_populate_remote_callbacks(git_remote_callbacks *callbacks);
 
 */
 import "C"
 import (
 	"crypto/x509"
+	"errors"
 	"reflect"
 	"runtime"
 	"strings"
@@ -219,43 +220,55 @@ func populateRemoteCallbacks(ptr *C.git_remote_callbacks, callbacks *RemoteCallb
 	if callbacks == nil {
 		return
 	}
-	C._go_git_setup_callbacks(ptr)
+	C._go_git_populate_remote_callbacks(ptr)
 	ptr.payload = pointerHandles.Track(callbacks)
 }
 
 //export sidebandProgressCallback
-func sidebandProgressCallback(_str *C.char, _len C.int, data unsafe.Pointer) int {
+func sidebandProgressCallback(errorMessage **C.char, _str *C.char, _len C.int, data unsafe.Pointer) C.int {
 	callbacks := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.SidebandProgressCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 	str := C.GoStringN(_str, _len)
-	return int(callbacks.SidebandProgressCallback(str))
+	ret := callbacks.SidebandProgressCallback(str)
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export completionCallback
-func completionCallback(completion_type C.git_remote_completion_type, data unsafe.Pointer) int {
+func completionCallback(errorMessage **C.char, completion_type C.git_remote_completion_type, data unsafe.Pointer) C.int {
 	callbacks := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.CompletionCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
-	return int(callbacks.CompletionCallback(RemoteCompletion(completion_type)))
+	ret := callbacks.CompletionCallback(RemoteCompletion(completion_type))
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export credentialsCallback
-func credentialsCallback(_cred **C.git_credential, _url *C.char, _username_from_url *C.char, allowed_types uint, data unsafe.Pointer) int {
+func credentialsCallback(
+	errorMessage **C.char,
+	_cred **C.git_credential,
+	_url *C.char,
+	_username_from_url *C.char,
+	allowed_types uint,
+	data unsafe.Pointer,
+) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.CredentialsCallback == nil {
-		return C.GIT_PASSTHROUGH
+		return C.int(ErrorCodePassthrough)
 	}
 	url := C.GoString(_url)
 	username_from_url := C.GoString(_username_from_url)
 	cred, err := callbacks.CredentialsCallback(url, username_from_url, (CredentialType)(allowed_types))
 	if err != nil {
-		if gitError, ok := err.(*GitError); ok {
+		return setCallbackError(errorMessage, err)
-			return int(gitError.Code)
-		}
-		return C.GIT_EUSER
 	}
 	if cred != nil {
 		*_cred = cred.ptr
@@ -264,41 +277,61 @@ func credentialsCallback(_cred **C.git_credential, _url *C.char, _username_from_
 		cred.ptr = nil
 		runtime.SetFinalizer(cred, nil)
 	}
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 //export transferProgressCallback
-func transferProgressCallback(stats *C.git_transfer_progress, data unsafe.Pointer) int {
+func transferProgressCallback(errorMessage **C.char, stats *C.git_transfer_progress, data unsafe.Pointer) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.TransferProgressCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
-	return int(callbacks.TransferProgressCallback(newTransferProgressFromC(stats)))
+	ret := callbacks.TransferProgressCallback(newTransferProgressFromC(stats))
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export updateTipsCallback
-func updateTipsCallback(_refname *C.char, _a *C.git_oid, _b *C.git_oid, data unsafe.Pointer) int {
+func updateTipsCallback(
+	errorMessage **C.char,
+	_refname *C.char,
+	_a *C.git_oid,
+	_b *C.git_oid,
+	data unsafe.Pointer,
+) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.UpdateTipsCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 	refname := C.GoString(_refname)
 	a := newOidFromC(_a)
 	b := newOidFromC(_b)
-	return int(callbacks.UpdateTipsCallback(refname, a, b))
+	ret := callbacks.UpdateTipsCallback(refname, a, b)
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export certificateCheckCallback
-func certificateCheckCallback(_cert *C.git_cert, _valid C.int, _host *C.char, data unsafe.Pointer) int {
+func certificateCheckCallback(
+	errorMessage **C.char,
+	_cert *C.git_cert,
+	_valid C.int,
+	_host *C.char,
+	data unsafe.Pointer,
+) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
 	// if there's no callback set, we need to make sure we fail if the library didn't consider this cert valid
 	if callbacks.CertificateCheckCallback == nil {
-		if _valid == 1 {
+		if _valid == 0 {
-			return 0
+			return C.int(ErrorCodeCertificate)
-		} else {
-			return C.GIT_ECERTIFICATE
 		}
+		return C.int(ErrorCodeOK)
 	}
+
 	host := C.GoString(_host)
 	valid := _valid != 0
 
@@ -308,7 +341,10 @@ func certificateCheckCallback(_cert *C.git_cert, _valid C.int, _host *C.char, da
 		ccert := (*C.git_cert_x509)(unsafe.Pointer(_cert))
 		x509_certs, err := x509.ParseCertificates(C.GoBytes(ccert.data, C.int(ccert.len)))
 		if err != nil {
-			return C.GIT_EUSER
+			return setCallbackError(errorMessage, err)
+		}
+		if len(x509_certs) < 1 {
+			return setCallbackError(errorMessage, errors.New("empty certificate list"))
 		}
 
 		// we assume there's only one, which should hold true for any web server we want to talk to
@@ -321,45 +357,56 @@ func certificateCheckCallback(_cert *C.git_cert, _valid C.int, _host *C.char, da
 		C.memcpy(unsafe.Pointer(&cert.Hostkey.HashSHA1[0]), unsafe.Pointer(&ccert.hash_sha1[0]), C.size_t(len(cert.Hostkey.HashSHA1)))
 		C.memcpy(unsafe.Pointer(&cert.Hostkey.HashSHA256[0]), unsafe.Pointer(&ccert.hash_sha256[0]), C.size_t(len(cert.Hostkey.HashSHA256)))
 	} else {
-		cstr := C.CString("Unsupported certificate type")
+		return setCallbackError(errorMessage, errors.New("unsupported certificate type"))
-		C.git_error_set_str(C.GITERR_NET, cstr)
-		C.free(unsafe.Pointer(cstr))
-		return -1 // we don't support anything else atm
 	}
 
-	return int(callbacks.CertificateCheckCallback(&cert, valid, host))
+	ret := callbacks.CertificateCheckCallback(&cert, valid, host)
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export packProgressCallback
-func packProgressCallback(stage C.int, current, total C.uint, data unsafe.Pointer) int {
+func packProgressCallback(errorMessage **C.char, stage C.int, current, total C.uint, data unsafe.Pointer) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
-
 	if callbacks.PackProgressCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 
-	return int(callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total)))
+	ret := callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total))
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export pushTransferProgressCallback
-func pushTransferProgressCallback(current, total C.uint, bytes C.size_t, data unsafe.Pointer) int {
+func pushTransferProgressCallback(errorMessage **C.char, current, total C.uint, bytes C.size_t, data unsafe.Pointer) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
 	if callbacks.PushTransferProgressCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 
-	return int(callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes)))
+	ret := callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes))
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 //export pushUpdateReferenceCallback
-func pushUpdateReferenceCallback(refname, status *C.char, data unsafe.Pointer) int {
+func pushUpdateReferenceCallback(errorMessage **C.char, refname, status *C.char, data unsafe.Pointer) C.int {
 	callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks)
-
 	if callbacks.PushUpdateReferenceCallback == nil {
-		return 0
+		return C.int(ErrorCodeOK)
 	}
 
-	return int(callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status)))
+	ret := callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status))
+	if ret < 0 {
+		return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String()))
+	}
+	return C.int(ErrorCodeOK)
 }
 
 func populateProxyOptions(ptr *C.git_proxy_options, opts *ProxyOptions) {
@@ -373,6 +420,10 @@ func populateProxyOptions(ptr *C.git_proxy_options, opts *ProxyOptions) {
 }
 
 func freeProxyOptions(ptr *C.git_proxy_options) {
+	if ptr == nil {
+		return
+	}
+
 	C.free(unsafe.Pointer(ptr.url))
 }
 

remote_test.go

@@ -30,7 +30,7 @@ func assertHostname(cert *Certificate, valid bool, hostname string, t *testing.T
 		return ErrorCodeUser
 	}
 
-	return 0
+	return ErrorCodeOK
 }
 
 func TestCertificateCheck(t *testing.T) {

reset.go

@@ -17,8 +17,15 @@ const (
 func (r *Repository) ResetToCommit(commit *Commit, resetType ResetType, opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
-	ret := C.git_reset(r.ptr, commit.ptr, C.git_reset_t(resetType), opts.toC())
 
+	var err error
+	cOpts := opts.toC(&err)
+	defer freeCheckoutOptions(cOpts)
+
+	ret := C.git_reset(r.ptr, commit.ptr, C.git_reset_t(resetType), cOpts)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}

revert.go

@@ -15,12 +15,15 @@ type RevertOptions struct {
 	CheckoutOpts CheckoutOptions
 }
 
-func (opts *RevertOptions) toC() *C.git_revert_options {
+func (opts *RevertOptions) toC(errorTarget *error) *C.git_revert_options {
+	if opts == nil {
+		return nil
+	}
 	return &C.git_revert_options{
 		version:       C.GIT_REVERT_OPTIONS_VERSION,
 		mainline:      C.uint(opts.Mainline),
 		merge_opts:    *opts.MergeOpts.toC(),
-		checkout_opts: *opts.CheckoutOpts.toC(),
+		checkout_opts: *opts.CheckoutOpts.toC(errorTarget),
 	}
 }
 
@@ -33,6 +36,10 @@ func revertOptionsFromC(opts *C.git_revert_options) RevertOptions {
 }
 
 func freeRevertOptions(opts *C.git_revert_options) {
+	if opts != nil {
+		return
+	}
+	freeMergeOptions(&opts.merge_opts)
 	freeCheckoutOptions(&opts.checkout_opts)
 }
 
@@ -57,19 +64,19 @@ func (r *Repository) Revert(commit *Commit, revertOptions *RevertOptions) error
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	var cOpts *C.git_revert_options
+	var err error
+	cOpts := revertOptions.toC(&err)
+	defer freeRevertOptions(cOpts)
 
-	if revertOptions != nil {
+	ret := C.git_revert(r.ptr, commit.cast_ptr, cOpts)
-		cOpts = revertOptions.toC()
-		defer freeRevertOptions(cOpts)
-	}
-
-	ecode := C.git_revert(r.ptr, commit.cast_ptr, cOpts)
 	runtime.KeepAlive(r)
 	runtime.KeepAlive(commit)
 
-	if ecode < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(ecode)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil
@@ -81,11 +88,8 @@ func (r *Repository) RevertCommit(revertCommit *Commit, ourCommit *Commit, mainl
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	var cOpts *C.git_merge_options
+	cOpts := mergeOptions.toC()
-
+	defer freeMergeOptions(cOpts)
-	if mergeOptions != nil {
-		cOpts = mergeOptions.toC()
-	}
 
 	var index *C.git_index
 

stash.go

@@ -3,7 +3,7 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_setup_stash_apply_progress_callbacks(git_stash_apply_options *opts);
+extern void _go_git_populate_stash_apply_callbacks(git_stash_apply_options *opts);
 extern int _go_git_stash_foreach(git_repository *repo, void *payload);
 */
 import "C"
@@ -113,28 +113,28 @@ const (
 
 // StashApplyProgressCallback is the apply operation notification callback.
 type StashApplyProgressCallback func(progress StashApplyProgress) error
-
+type stashApplyProgressCallbackData struct {
-type stashApplyProgressData struct {
+	callback    StashApplyProgressCallback
-	Callback StashApplyProgressCallback
+	errorTarget *error
-	Error    error
 }
 
-//export stashApplyProgressCb
+//export stashApplyProgressCallback
-func stashApplyProgressCb(progress C.git_stash_apply_progress_t, handle unsafe.Pointer) int {
+func stashApplyProgressCallback(progress C.git_stash_apply_progress_t, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*stashApplyProgressData)
+	data, ok := payload.(*stashApplyProgressCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
-
+	if data == nil || data.callback == nil {
-	if data != nil {
+		return C.int(ErrorCodeOK)
-		err := data.Callback(StashApplyProgress(progress))
-		if err != nil {
-			data.Error = err
-			return C.GIT_EUSER
-		}
 	}
-	return 0
+
+	err := data.callback(StashApplyProgress(progress))
+	if err != nil {
+		*data.errorTarget = err
+		return C.int(ErrorCodeUser)
+	}
+	return C.int(ErrorCodeOK)
 }
 
 // StashApplyOptions represents options to control the apply operation.
@@ -161,38 +161,34 @@ func DefaultStashApplyOptions() (StashApplyOptions, error) {
 	}, nil
 }
 
-func (opts *StashApplyOptions) toC() (
+func (opts *StashApplyOptions) toC(errorTarget *error) *C.git_stash_apply_options {
-	optsC *C.git_stash_apply_options, progressData *stashApplyProgressData) {
+	if opts == nil {
-
+		return nil
-	if opts != nil {
-		progressData = &stashApplyProgressData{
-			Callback: opts.ProgressCallback,
-		}
-
-		optsC = &C.git_stash_apply_options{
-			version: C.GIT_STASH_APPLY_OPTIONS_VERSION,
-			flags:   C.uint32_t(opts.Flags),
-		}
-		populateCheckoutOptions(&optsC.checkout_options, &opts.CheckoutOptions)
-		if opts.ProgressCallback != nil {
-			C._go_git_setup_stash_apply_progress_callbacks(optsC)
-			optsC.progress_payload = pointerHandles.Track(progressData)
-		}
 	}
-	return
+	optsC := &C.git_stash_apply_options{
-}
+		version: C.GIT_STASH_APPLY_OPTIONS_VERSION,
-
+		flags:   C.uint32_t(opts.Flags),
-// should be called after every call to toC() as deferred.
-func untrackStashApplyOptionsCallback(optsC *C.git_stash_apply_options) {
-	if optsC != nil && optsC.progress_payload != nil {
-		pointerHandles.Untrack(optsC.progress_payload)
 	}
+	populateCheckoutOptions(&optsC.checkout_options, &opts.CheckoutOptions, errorTarget)
+	if opts.ProgressCallback != nil {
+		progressData := &stashApplyProgressCallbackData{
+			callback:    opts.ProgressCallback,
+			errorTarget: errorTarget,
+		}
+		C._go_git_populate_stash_apply_callbacks(optsC)
+		optsC.progress_payload = pointerHandles.Track(progressData)
+	}
+	return optsC
 }
 
 func freeStashApplyOptions(optsC *C.git_stash_apply_options) {
-	if optsC != nil {
+	if optsC == nil {
-		freeCheckoutOptions(&optsC.checkout_options)
+		return
 	}
+	if optsC.progress_payload != nil {
+		pointerHandles.Untrack(optsC.progress_payload)
+	}
+	freeCheckoutOptions(&optsC.checkout_options)
 }
 
 // Apply applies a single stashed state from the stash list.
@@ -220,8 +216,8 @@ func freeStashApplyOptions(optsC *C.git_stash_apply_options) {
 //
 // Error codes can be interogated with IsErrorCode(err, ErrorCodeNotFound).
 func (c *StashCollection) Apply(index int, opts StashApplyOptions) error {
-	optsC, progressData := opts.toC()
+	var err error
-	defer untrackStashApplyOptionsCallback(optsC)
+	optsC := opts.toC(&err)
 	defer freeStashApplyOptions(optsC)
 
 	runtime.LockOSThread()
@@ -229,8 +225,8 @@ func (c *StashCollection) Apply(index int, opts StashApplyOptions) error {
 
 	ret := C.git_stash_apply(c.repo.ptr, C.size_t(index), optsC)
 	runtime.KeepAlive(c)
-	if ret == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return progressData.Error
+		return err
 	}
 	if ret < 0 {
 		return MakeGitError(ret)
@@ -245,26 +241,25 @@ func (c *StashCollection) Apply(index int, opts StashApplyOptions) error {
 // 'message' is the message used when creating the stash and 'id'
 // is the commit id of the stash.
 type StashCallback func(index int, message string, id *Oid) error
-
 type stashCallbackData struct {
-	Callback StashCallback
+	callback    StashCallback
-	Error    error
+	errorTarget *error
 }
 
-//export stashForeachCb
+//export stashForeachCallback
-func stashForeachCb(index C.size_t, message *C.char, id *C.git_oid, handle unsafe.Pointer) int {
+func stashForeachCallback(index C.size_t, message *C.char, id *C.git_oid, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
 	data, ok := payload.(*stashCallbackData)
 	if !ok {
 		panic("could not retrieve data for handle")
 	}
 
-	err := data.Callback(int(index), C.GoString(message), newOidFromC(id))
+	err := data.callback(int(index), C.GoString(message), newOidFromC(id))
 	if err != nil {
-		data.Error = err
+		*data.errorTarget = err
-		return C.GIT_EUSER
+		return C.int(ErrorCodeUser)
 	}
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 // Foreach loops over all the stashed states and calls the callback
@@ -272,10 +267,11 @@ func stashForeachCb(index C.size_t, message *C.char, id *C.git_oid, handle unsaf
 //
 // If callback returns an error, this will stop looping.
 func (c *StashCollection) Foreach(callback StashCallback) error {
+	var err error
 	data := stashCallbackData{
-		Callback: callback,
+		callback:    callback,
+		errorTarget: &err,
 	}
-
 	handle := pointerHandles.Track(&data)
 	defer pointerHandles.Untrack(handle)
 
@@ -284,8 +280,8 @@ func (c *StashCollection) Foreach(callback StashCallback) error {
 
 	ret := C._go_git_stash_foreach(c.repo.ptr, handle)
 	runtime.KeepAlive(c)
-	if ret == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return data.Error
+		return err
 	}
 	if ret < 0 {
 		return MakeGitError(ret)
@@ -323,8 +319,8 @@ func (c *StashCollection) Drop(index int) error {
 // Returns error code ErrorCodeNotFound if there's no stashed
 // state for the given index.
 func (c *StashCollection) Pop(index int, opts StashApplyOptions) error {
-	optsC, progressData := opts.toC()
+	var err error
-	defer untrackStashApplyOptionsCallback(optsC)
+	optsC := opts.toC(&err)
 	defer freeStashApplyOptions(optsC)
 
 	runtime.LockOSThread()
@@ -332,8 +328,8 @@ func (c *StashCollection) Pop(index int, opts StashApplyOptions) error {
 
 	ret := C.git_stash_pop(c.repo.ptr, C.size_t(index), optsC)
 	runtime.KeepAlive(c)
-	if ret == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return progressData.Error
+		return err
 	}
 	if ret < 0 {
 		return MakeGitError(ret)

submodule.go

@@ -6,7 +6,9 @@ package git
 extern int _go_git_visit_submodule(git_repository *repo, void *fct);
 */
 import "C"
+
 import (
+	"errors"
 	"runtime"
 	"unsafe"
 )
@@ -108,30 +110,50 @@ func (c *SubmoduleCollection) Lookup(name string) (*Submodule, error) {
 
 // SubmoduleCallback is a function that is called for every submodule found in SubmoduleCollection.Foreach.
 type SubmoduleCallback func(sub *Submodule, name string) int
+type submoduleCallbackData struct {
+	callback    SubmoduleCallback
+	errorTarget *error
+}
 
 //export submoduleCallback
 func submoduleCallback(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer) C.int {
 	sub := &Submodule{(*C.git_submodule)(csub), nil}
 
-	if callback, ok := pointerHandles.Get(handle).(SubmoduleCallback); ok {
+	data, ok := pointerHandles.Get(handle).(submoduleCallbackData)
-		return (C.int)(callback(sub, C.GoString(name)))
+	if !ok {
-	} else {
 		panic("invalid submodule visitor callback")
 	}
+
+	ret := data.callback(sub, C.GoString(name))
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+
+	return C.int(ErrorCodeOK)
 }
 
-func (c *SubmoduleCollection) Foreach(cbk SubmoduleCallback) error {
+func (c *SubmoduleCollection) Foreach(callback SubmoduleCallback) error {
+	var err error
+	data := submoduleCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	handle := pointerHandles.Track(cbk)
+	handle := pointerHandles.Track(data)
 	defer pointerHandles.Untrack(handle)
 
 	ret := C._go_git_visit_submodule(c.repo.ptr, handle)
 	runtime.KeepAlive(c)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
@@ -342,17 +364,18 @@ func (sub *Submodule) Open() (*Repository, error) {
 }
 
 func (sub *Submodule) Update(init bool, opts *SubmoduleUpdateOptions) error {
-	var copts C.git_submodule_update_options
+	var err error
-	err := populateSubmoduleUpdateOptions(&copts, opts)
+	cOpts := populateSubmoduleUpdateOptions(&C.git_submodule_update_options{}, opts, &err)
-	if err != nil {
+	defer freeSubmoduleUpdateOptions(cOpts)
-		return err
-	}
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ret := C.git_submodule_update(sub.ptr, cbool(init), &copts)
+	ret := C.git_submodule_update(sub.ptr, cbool(init), cOpts)
 	runtime.KeepAlive(sub)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
@@ -360,15 +383,22 @@ func (sub *Submodule) Update(init bool, opts *SubmoduleUpdateOptions) error {
 	return nil
 }
 
-func populateSubmoduleUpdateOptions(ptr *C.git_submodule_update_options, opts *SubmoduleUpdateOptions) error {
+func populateSubmoduleUpdateOptions(ptr *C.git_submodule_update_options, opts *SubmoduleUpdateOptions, errorTarget *error) *C.git_submodule_update_options {
 	C.git_submodule_update_options_init(ptr, C.GIT_SUBMODULE_UPDATE_OPTIONS_VERSION)
 
 	if opts == nil {
 		return nil
 	}
 
-	populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts)
+	populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts, errorTarget)
 	populateFetchOptions(&ptr.fetch_opts, opts.FetchOptions)
 
-	return nil
+	return ptr
+}
+
+func freeSubmoduleUpdateOptions(ptr *C.git_submodule_update_options) {
+	if ptr == nil {
+		return
+	}
+	freeCheckoutOptions(&ptr.checkout_opts)
 }

tag.go

@@ -197,48 +197,48 @@ func (c *TagsCollection) ListWithMatch(pattern string) ([]string, error) {
 // so repo.LookupTag() will return an error for these tags. Use
 // repo.References.Lookup() instead.
 type TagForeachCallback func(name string, id *Oid) error
-type tagForeachData struct {
+type tagForeachCallbackData struct {
-	callback TagForeachCallback
+	callback    TagForeachCallback
-	err      error
+	errorTarget *error
 }
 
-//export gitTagForeachCb
+//export tagForeachCallback
-func gitTagForeachCb(name *C.char, id *C.git_oid, handle unsafe.Pointer) int {
+func tagForeachCallback(name *C.char, id *C.git_oid, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*tagForeachData)
+	data, ok := payload.(*tagForeachCallbackData)
 	if !ok {
 		panic("could not retrieve tag foreach CB handle")
 	}
 
 	err := data.callback(C.GoString(name), newOidFromC(id))
 	if err != nil {
-		data.err = err
+		*data.errorTarget = err
-		return C.GIT_EUSER
+		return C.int(ErrorCodeUser)
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 // Foreach calls the callback for each tag in the repository.
 func (c *TagsCollection) Foreach(callback TagForeachCallback) error {
-	data := tagForeachData{
+	var err error
-		callback: callback,
+	data := tagForeachCallbackData{
-		err:      nil,
+		callback:    callback,
+		errorTarget: &err,
 	}
-
 	handle := pointerHandles.Track(&data)
 	defer pointerHandles.Untrack(handle)
 
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	err := C._go_git_tag_foreach(c.repo.ptr, handle)
+	ret := C._go_git_tag_foreach(c.repo.ptr, handle)
 	runtime.KeepAlive(c)
-	if err == C.GIT_EUSER {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return data.err
+		return err
 	}
-	if err < 0 {
+	if ret < 0 {
-		return MakeGitError(err)
+		return MakeGitError(ret)
 	}
 
 	return nil

tree.go

@@ -8,6 +8,7 @@ extern int _go_git_treewalk(git_tree *tree, git_treewalk_mode mode, void *ptr);
 import "C"
 
 import (
+	"errors"
 	"runtime"
 	"unsafe"
 )
@@ -120,33 +121,46 @@ func (t *Tree) EntryCount() uint64 {
 }
 
 type TreeWalkCallback func(string, *TreeEntry) int
+type treeWalkCallbackData struct {
+	callback    TreeWalkCallback
+	errorTarget *error
+}
 
 //export treeWalkCallback
 func treeWalkCallback(_root *C.char, entry *C.git_tree_entry, ptr unsafe.Pointer) C.int {
-	root := C.GoString(_root)
+	data, ok := pointerHandles.Get(ptr).(*treeWalkCallbackData)
-
+	if !ok {
-	if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok {
-		return C.int(callback(root, newTreeEntry(entry)))
-	} else {
 		panic("invalid treewalk callback")
 	}
+
+	ret := data.callback(C.GoString(_root), newTreeEntry(entry))
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+
+	return C.int(ErrorCodeOK)
 }
 
 func (t *Tree) Walk(callback TreeWalkCallback) error {
+	var err error
+	data := treeWalkCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	ptr := pointerHandles.Track(callback)
+	handle := pointerHandles.Track(&data)
-	defer pointerHandles.Untrack(ptr)
+	defer pointerHandles.Untrack(handle)
 
-	err := C._go_git_treewalk(
+	ret := C._go_git_treewalk(t.cast_ptr, C.GIT_TREEWALK_PRE, handle)
-		t.cast_ptr,
-		C.GIT_TREEWALK_PRE,
-		ptr,
-	)
 	runtime.KeepAlive(t)
-	if err < 0 {
+	if ret == C.int(ErrorCodeUser) && err != nil {
-		return MakeGitError(err)
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil

wrapper.c

@@ -1,36 +1,149 @@
 #include "_cgo_export.h"
+
 #include <git2.h>
 #include <git2/sys/odb_backend.h>
 #include <git2/sys/refdb_backend.h>
 #include <git2/sys/cred.h>
 
-typedef int (*gogit_submodule_cbk)(git_submodule *sm, const char *name, void *payload);
+// There are two ways in which to declare a callback:
+//
+// * If there is a guarantee that the callback will always be called within the
+//   same stack (e.g. by passing the callback directly into a function / into a
+//   struct that goes into a function), the following pattern is preferred,
+//   which preserves the error object as-is:
+//
+//   // myfile.go
+//   type FooCallback func(...) (..., error)
+//   type FooCallbackData struct {
+//     callback    FooCallback
+//     errorTarget *error
+//   }
+//
+//   //export fooCallback
+//   func fooCallback(..., handle unsafe.Pointer) C.int {
+//     payload := pointerHandles.Get(handle)
+//     data := payload.(*fooCallbackData)
+//     ...
+//     err := data.callback(...)
+//     if err != nil {
+//       *data.errorTarget = err
+//       return C.int(ErrorCodeUser)
+//     }
+//     return C.int(ErrorCodeOK)
+//   }
+//
+//   func MyFunction(... callback FooCallback) error {
+//    var err error
+//    data := fooCallbackData{
+//      callback:    callback,
+//      errorTarget: &err,
+//    }
+//    handle := pointerHandles.Track(&data)
+//    defer pointerHandles.Untrack(handle)
+//
+//    runtime.LockOSThread()
+//    defer runtime.UnlockOSThread()
+//
+//    ret := C._go_git_my_function(..., handle)
+//    if ret == C.int(ErrorCodeUser) && err != nil {
+//      return err
+//    }
+//    if ret < 0 {
+//      return MakeGitError(ret)
+//    }
+//    return nil
+//   }
+//
+//   // wrapper.c
+//   int _go_git_my_function(..., void *payload)
+//   {
+//     return git_my_function(..., (git_foo_cb)&fooCallback, payload);
+//   }
+//
+// * Otherwise, if the same callback can be invoked from multiple functions or
+//   from different stacks (e.g. when passing the callback to an object), a
+//   different pattern should be used instead, which has the downside of losing
+//   the original error object and converting it to a GitError:
+//
+//   // myfile.go
+//   type FooCallback func(...) (..., error)
+//
+//   //export fooCallback
+//   func fooCallback(errorMessage **C.char, ..., handle unsafe.Pointer) C.int {
+//     callback := pointerHandles.Get(data).(*FooCallback)
+//     ...
+//     err := callback(...)
+//     if err != nil {
+//       return setCallbackError(errorMessage, err)
+//     }
+//     return C.int(ErrorCodeOK)
+//   }
+//
+//   // wrapper.c
+//   static int foo_callback(...)
+//   {
+//     char *error_message = NULL;
+//     const int ret = fooCallback(&error_message, ...);
+//     return set_callback_error(error_message, ret);
+//   }
 
-void _go_git_populate_apply_cb(git_apply_options *options)
+/**
+ * Sets the thread-local error to the provided string. This needs to happen in
+ * C because Go might change Goroutines _just_ before returning, which would
+ * lose the contents of the error message.
+ */
+static int set_callback_error(char *error_message, int ret)
 {
-	options->delta_cb = (git_apply_delta_cb)deltaApplyCallback;
+	if (error_message != NULL) {
-	options->hunk_cb = (git_apply_hunk_cb)hunkApplyCallback;
+		if (ret < 0)
+			git_error_set_str(GIT_ERROR_CALLBACK, error_message);
+		free(error_message);
+	}
+	return ret;
 }
 
-void _go_git_populate_commit_sign_cb(git_rebase_options *opts)
+void _go_git_populate_apply_callbacks(git_apply_options *options)
 {
-	opts->signing_cb = (git_commit_signing_cb)commitSignCallback;
+	options->delta_cb = (git_apply_delta_cb)&deltaApplyCallback;
+	options->hunk_cb = (git_apply_hunk_cb)&hunkApplyCallback;
 }
 
-void _go_git_populate_remote_cb(git_clone_options *opts)
+static int commit_signing_callback(
+		git_buf *signature,
+		git_buf *signature_field,
+		const char *commit_contents,
+		void *payload)
 {
-	opts->remote_cb = (git_remote_create_cb)remoteCreateCallback;
+	char *error_message = NULL;
+	const int ret = commitSigningCallback(
+			&error_message,
+			signature,
+			signature_field,
+			(char *)commit_contents,
+			payload
+	);
+	return set_callback_error(error_message, ret);
 }
 
-void _go_git_populate_checkout_cb(git_checkout_options *opts)
+void _go_git_populate_rebase_callbacks(git_rebase_options *opts)
 {
-	opts->notify_cb = (git_checkout_notify_cb)checkoutNotifyCallback;
+	opts->signing_cb = commit_signing_callback;
-	opts->progress_cb = (git_checkout_progress_cb)checkoutProgressCallback;
+}
+
+void _go_git_populate_clone_callbacks(git_clone_options *opts)
+{
+	opts->remote_cb = (git_remote_create_cb)&remoteCreateCallback;
+}
+
+void _go_git_populate_checkout_callbacks(git_checkout_options *opts)
+{
+	opts->notify_cb = (git_checkout_notify_cb)&checkoutNotifyCallback;
+	opts->progress_cb = (git_checkout_progress_cb)&checkoutProgressCallback;
 }
 
 int _go_git_visit_submodule(git_repository *repo, void *fct)
 {
-	  return git_submodule_foreach(repo, (gogit_submodule_cbk)&submoduleCallback, fct);
+	return git_submodule_foreach(repo, (git_submodule_cb)&submoduleCallback, fct);
 }
 
 int _go_git_treewalk(git_tree *tree, git_treewalk_mode mode, void *ptr)
@@ -40,28 +153,26 @@ int _go_git_treewalk(git_tree *tree, git_treewalk_mode mode, void *ptr)
 
 int _go_git_packbuilder_foreach(git_packbuilder *pb, void *payload)
 {
-    return git_packbuilder_foreach(pb, (git_packbuilder_foreach_cb)&packbuilderForEachCb, payload);
+	return git_packbuilder_foreach(pb, (git_packbuilder_foreach_cb)&packbuilderForEachCallback, payload);
 }
 
 int _go_git_odb_foreach(git_odb *db, void *payload)
 {
-    return git_odb_foreach(db, (git_odb_foreach_cb)&odbForEachCb, payload);
+	return git_odb_foreach(db, (git_odb_foreach_cb)&odbForEachCallback, payload);
 }
 
 void _go_git_odb_backend_free(git_odb_backend *backend)
 {
-    if (backend->free)
+	if (!backend->free)
-      backend->free(backend);
+		return;
-
+	backend->free(backend);
-    return;
 }
 
 void _go_git_refdb_backend_free(git_refdb_backend *backend)
 {
-    if (backend->free)
+	if (!backend->free)
-      backend->free(backend);
+		return;
-
+	backend->free(backend);
-    return;
 }
 
 int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLine, void *payload)
@@ -70,83 +181,210 @@ int _go_git_diff_foreach(git_diff *diff, int eachFile, int eachHunk, int eachLin
 	git_diff_hunk_cb hcb = NULL;
 	git_diff_line_cb lcb = NULL;
 
-	if (eachFile) {
+	if (eachFile)
-		fcb = (git_diff_file_cb)&diffForEachFileCb;
+		fcb = (git_diff_file_cb)&diffForEachFileCallback;
-	}
+	if (eachHunk)
-
+		hcb = (git_diff_hunk_cb)&diffForEachHunkCallback;
-	if (eachHunk) {
+	if (eachLine)
-		hcb = (git_diff_hunk_cb)&diffForEachHunkCb;
+		lcb = (git_diff_line_cb)&diffForEachLineCallback;
-	}
-
-	if (eachLine) {
-		lcb = (git_diff_line_cb)&diffForEachLineCb;
-	}
 
 	return git_diff_foreach(diff, fcb, NULL, hcb, lcb, payload);
 }
 
-int _go_git_diff_blobs(git_blob *old, const char *old_path, git_blob *new, const char *new_path, git_diff_options *opts, int eachFile, int eachHunk, int eachLine, void *payload)
+int _go_git_diff_blobs(
+		git_blob *old,
+		const char *old_path,
+		git_blob *new,
+		const char *new_path,
+		git_diff_options *opts,
+		int eachFile,
+		int eachHunk,
+		int eachLine,
+		void *payload)
 {
 	git_diff_file_cb fcb = NULL;
 	git_diff_hunk_cb hcb = NULL;
 	git_diff_line_cb lcb = NULL;
 
-	if (eachFile) {
+	if (eachFile)
-		fcb = (git_diff_file_cb)&diffForEachFileCb;
+		fcb = (git_diff_file_cb)&diffForEachFileCallback;
-	}
+	if (eachHunk)
-
+		hcb = (git_diff_hunk_cb)&diffForEachHunkCallback;
-	if (eachHunk) {
+	if (eachLine)
-		hcb = (git_diff_hunk_cb)&diffForEachHunkCb;
+		lcb = (git_diff_line_cb)&diffForEachLineCallback;
-	}
-
-	if (eachLine) {
-		lcb = (git_diff_line_cb)&diffForEachLineCb;
-	}
 
 	return git_diff_blobs(old, old_path, new, new_path, opts, fcb, NULL, hcb, lcb, payload);
 }
 
-void _go_git_setup_diff_notify_callbacks(git_diff_options *opts) {
+void _go_git_setup_diff_notify_callbacks(git_diff_options *opts)
-	opts->notify_cb = (git_diff_notify_cb)diffNotifyCb;
+{
+	opts->notify_cb = (git_diff_notify_cb)&diffNotifyCallback;
 }
 
-void _go_git_setup_callbacks(git_remote_callbacks *callbacks) {
+static int sideband_progress_callback(const char *str, int len, void *payload)
-	typedef int (*completion_cb)(git_remote_completion_type type, void *data);
+{
-	typedef int (*update_tips_cb)(const char *refname, const git_oid *a, const git_oid *b, void *data);
+	char *error_message = NULL;
-	typedef int (*push_update_reference_cb)(const char *refname, const char *status, void *data);
+	const int ret = sidebandProgressCallback(&error_message, (char *)str, len, payload);
-
+	return set_callback_error(error_message, ret);
-	callbacks->sideband_progress = (git_transport_message_cb)sidebandProgressCallback;
-	callbacks->completion = (completion_cb)completionCallback;
-	callbacks->credentials = (git_credential_acquire_cb)credentialsCallback;
-	callbacks->transfer_progress = (git_transfer_progress_cb)transferProgressCallback;
-	callbacks->update_tips = (update_tips_cb)updateTipsCallback;
-	callbacks->certificate_check = (git_transport_certificate_check_cb) certificateCheckCallback;
-	callbacks->pack_progress = (git_packbuilder_progress) packProgressCallback;
-	callbacks->push_transfer_progress = (git_push_transfer_progress) pushTransferProgressCallback;
-	callbacks->push_update_reference = (push_update_reference_cb) pushUpdateReferenceCallback;
 }
 
-int _go_git_index_add_all(git_index *index, const git_strarray *pathspec, unsigned int flags, void *callback) {
+static int completion_callback(git_remote_completion_type completion_type, void *data)
-	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb) &indexMatchedPathCallback : NULL;
+{
+	char *error_message = NULL;
+	const int ret = completionCallback(&error_message, completion_type, data);
+	return set_callback_error(error_message, ret);
+}
+
+static int credentials_callback(
+		git_credential **cred,
+		const char *url,
+		const char *username_from_url,
+		unsigned int allowed_types,
+		void *data)
+{
+	char *error_message = NULL;
+	const int ret = credentialsCallback(
+			&error_message,
+			cred,
+			(char *)url,
+			(char *)username_from_url,
+			allowed_types,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int transfer_progress_callback(const git_transfer_progress *stats, void *data)
+{
+	char *error_message = NULL;
+	const int ret = transferProgressCallback(
+			&error_message,
+			(git_transfer_progress *)stats,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int update_tips_callback(const char *refname, const git_oid *a, const git_oid *b, void *data)
+{
+	char *error_message = NULL;
+	const int ret = updateTipsCallback(
+			&error_message,
+			(char *)refname,
+			(git_oid *)a,
+			(git_oid *)b,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int certificate_check_callback(git_cert *cert, int valid, const char *host, void *data)
+{
+	char *error_message = NULL;
+	const int ret = certificateCheckCallback(
+			&error_message,
+			cert,
+			valid,
+			(char *)host,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int pack_progress_callback(int stage, unsigned int current, unsigned int total, void *data)
+{
+	char *error_message = NULL;
+	const int ret = packProgressCallback(
+			&error_message,
+			stage,
+			current,
+			total,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int push_transfer_progress_callback(
+		unsigned int current,
+		unsigned int total,
+		size_t bytes,
+		void *data)
+{
+	char *error_message = NULL;
+	const int ret = pushTransferProgressCallback(
+			&error_message,
+			current,
+			total,
+			bytes,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+static int push_update_reference_callback(const char *refname, const char *status, void *data)
+{
+	char *error_message = NULL;
+	const int ret = pushUpdateReferenceCallback(
+			&error_message,
+			(char *)refname,
+			(char *)status,
+			data
+	);
+	return set_callback_error(error_message, ret);
+}
+
+void _go_git_populate_remote_callbacks(git_remote_callbacks *callbacks)
+{
+	callbacks->sideband_progress = sideband_progress_callback;
+	callbacks->completion = completion_callback;
+	callbacks->credentials = credentials_callback;
+	callbacks->transfer_progress = transfer_progress_callback;
+	callbacks->update_tips = update_tips_callback;
+	callbacks->certificate_check = certificate_check_callback;
+	callbacks->pack_progress = pack_progress_callback;
+	callbacks->push_transfer_progress = push_transfer_progress_callback;
+	callbacks->push_update_reference = push_update_reference_callback;
+}
+
+int _go_git_index_add_all(git_index *index, const git_strarray *pathspec, unsigned int flags, void *callback)
+{
+	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb)&indexMatchedPathCallback : NULL;
 	return git_index_add_all(index, pathspec, flags, cb, callback);
 }
 
-int _go_git_index_update_all(git_index *index, const git_strarray *pathspec, void *callback) {
+int _go_git_index_update_all(git_index *index, const git_strarray *pathspec, void *callback)
-	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb) &indexMatchedPathCallback : NULL;
+{
+	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb)&indexMatchedPathCallback : NULL;
 	return git_index_update_all(index, pathspec, cb, callback);
 }
 
-int _go_git_index_remove_all(git_index *index, const git_strarray *pathspec, void *callback) {
+int _go_git_index_remove_all(git_index *index, const git_strarray *pathspec, void *callback)
-	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb) &indexMatchedPathCallback : NULL;
+{
+	git_index_matched_path_cb cb = callback ? (git_index_matched_path_cb)&indexMatchedPathCallback : NULL;
 	return git_index_remove_all(index, pathspec, cb, callback);
 }
 
 int _go_git_tag_foreach(git_repository *repo, void *payload)
 {
-    return git_tag_foreach(repo, (git_tag_foreach_cb)&gitTagForeachCb, payload);
+	return git_tag_foreach(repo, (git_tag_foreach_cb)&tagForeachCallback, payload);
 }
 
-int _go_git_merge_file(git_merge_file_result* out, char* ancestorContents, size_t ancestorLen, char* ancestorPath, unsigned int ancestorMode, char* oursContents, size_t oursLen, char* oursPath, unsigned int oursMode, char* theirsContents, size_t theirsLen, char* theirsPath, unsigned int theirsMode, git_merge_file_options* copts) {
+int _go_git_merge_file(
+		git_merge_file_result* out,
+		char* ancestorContents,
+		size_t ancestorLen,
+		char* ancestorPath,
+		unsigned int ancestorMode,
+		char* oursContents,
+		size_t oursLen,
+		char* oursPath,
+		unsigned int oursMode,
+		char* theirsContents,
+		size_t theirsLen,
+		char* theirsPath,
+		unsigned int theirsMode,
+		git_merge_file_options* copts)
+{
 	git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT;
 	git_merge_file_input ours = GIT_MERGE_FILE_INPUT_INIT;
 	git_merge_file_input theirs = GIT_MERGE_FILE_INPUT_INIT;
@@ -169,12 +407,14 @@ int _go_git_merge_file(git_merge_file_result* out, char* ancestorContents, size_
 	return git_merge_file(out, &ancestor, &ours, &theirs, copts);
 }
 
-void _go_git_setup_stash_apply_progress_callbacks(git_stash_apply_options *opts) {
+void _go_git_populate_stash_apply_callbacks(git_stash_apply_options *opts)
-	opts->progress_cb = (git_stash_apply_progress_cb)stashApplyProgressCb;
+{
+	opts->progress_cb = (git_stash_apply_progress_cb)&stashApplyProgressCallback;
 }
 
-int _go_git_stash_foreach(git_repository *repo, void *payload) {
+int _go_git_stash_foreach(git_repository *repo, void *payload)
-    return git_stash_foreach(repo, (git_stash_cb)&stashForeachCb, payload);
+{
+	return git_stash_foreach(repo, (git_stash_cb)&stashForeachCallback, payload);
 }
 
 int _go_git_writestream_write(git_writestream *stream, const char *buffer, size_t len)
@@ -192,16 +432,21 @@ void _go_git_writestream_free(git_writestream *stream)
 	stream->free(stream);
 }
 
-git_credential_t _go_git_credential_credtype(git_credential *cred) {
+git_credential_t _go_git_credential_credtype(git_credential *cred)
+{
 	return cred->credtype;
 }
 
 int _go_git_odb_write_pack(git_odb_writepack **out, git_odb *db, void *progress_payload)
 {
-	return git_odb_write_pack(out, db, (git_transfer_progress_cb)transferProgressCallback, progress_payload);
+	return git_odb_write_pack(out, db, transfer_progress_callback, progress_payload);
 }
 
-int _go_git_odb_writepack_append(git_odb_writepack *writepack, const void *data, size_t size, git_transfer_progress *stats)
+int _go_git_odb_writepack_append(
+		git_odb_writepack *writepack,
+		const void *data,
+		size_t size,
+		git_transfer_progress *stats)
 {
 	return writepack->append(writepack, data, size, stats);
 }
@@ -216,12 +461,15 @@ void _go_git_odb_writepack_free(git_odb_writepack *writepack)
 	writepack->free(writepack);
 }
 
-int _go_git_indexer_new(git_indexer **out, const char *path, unsigned int mode, git_odb *odb, void *progress_cb_payload)
+int _go_git_indexer_new(
+		git_indexer **out,
+		const char *path,
+		unsigned int mode,
+		git_odb *odb,
+		void *progress_cb_payload)
 {
 	git_indexer_options indexer_options = GIT_INDEXER_OPTIONS_INIT;
-	indexer_options.progress_cb = (git_transfer_progress_cb)transferProgressCallback;
+	indexer_options.progress_cb = transfer_progress_callback;
 	indexer_options.progress_cb_payload = progress_cb_payload;
 	return git_indexer_new(out, path, mode, odb, &indexer_options);
 }
-
-/* EOF */