Fix the Cred interface

This change adds Cred.Free() and finalizers to prevent memory leaks. It
also makes the interface for Cred more idiomatic and return actual
errors intead of ints.

2 files changed, 73 insertions, 21 deletions

diff --git a/credentials.go b/credentials.go
index 038313b..5469b20 100644
--- a/credentials.go
+++ b/credentials.go
@@ -7,7 +7,10 @@ package git
 git_credtype_t _go_git_cred_credtype(git_cred *cred);
 */
 import "C"
-import "unsafe"
+import (
+	"runtime"
+	"unsafe"
+)
 
 type CredType uint
 
@@ -22,6 +25,12 @@ type Cred struct {
 	ptr *C.git_cred
 }
 
+func newCred() *Cred {
+	cred := &Cred{}
+	runtime.SetFinalizer(cred, (*Cred).Free)
+	return cred
+}
+
 func (o *Cred) HasUsername() bool {
 	if C.git_cred_has_username(o.ptr) == 1 {
 		return true
@@ -33,24 +42,36 @@ func (o *Cred) Type() CredType {
 	return (CredType)(C._go_git_cred_credtype(o.ptr))
 }
 
-func credFromC(ptr *C.git_cred) *Cred {
-	return &Cred{ptr}
+func (o *Cred) Free() {
+	C.git_cred_free(o.ptr)
+	runtime.SetFinalizer(o, nil)
+	o.ptr = nil
 }
 
-func NewCredUserpassPlaintext(username string, password string) (int, Cred) {
-	cred := Cred{}
+func NewCredUserpassPlaintext(username string, password string) (*Cred, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cred := newCred()
 	cusername := C.CString(username)
 	defer C.free(unsafe.Pointer(cusername))
 	cpassword := C.CString(password)
 	defer C.free(unsafe.Pointer(cpassword))
 	ret := C.git_cred_userpass_plaintext_new(&cred.ptr, cusername, cpassword)
-	return int(ret), cred
+	if ret != 0 {
+		cred.Free()
+		return nil, MakeGitError(ret)
+	}
+	return cred, nil
 }
 
 // NewCredSshKey creates new ssh credentials reading the public and private keys
 // from the file system.
-func NewCredSshKey(username string, publicKeyPath string, privateKeyPath string, passphrase string) (int, Cred) {
-	cred := Cred{}
+func NewCredSshKey(username string, publicKeyPath string, privateKeyPath string, passphrase string) (*Cred, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cred := newCred()
 	cusername := C.CString(username)
 	defer C.free(unsafe.Pointer(cusername))
 	cpublickey := C.CString(publicKeyPath)
@@ -60,13 +81,20 @@ func NewCredSshKey(username string, publicKeyPath string, privateKeyPath string,
 	cpassphrase := C.CString(passphrase)
 	defer C.free(unsafe.Pointer(cpassphrase))
 	ret := C.git_cred_ssh_key_new(&cred.ptr, cusername, cpublickey, cprivatekey, cpassphrase)
-	return int(ret), cred
+	if ret != 0 {
+		cred.Free()
+		return nil, MakeGitError(ret)
+	}
+	return cred, nil
 }
 
 // NewCredSshKeyFromMemory creates new ssh credentials using the publicKey and privateKey
 // arguments as the values for the public and private keys.
-func NewCredSshKeyFromMemory(username string, publicKey string, privateKey string, passphrase string) (int, Cred) {
-	cred := Cred{}
+func NewCredSshKeyFromMemory(username string, publicKey string, privateKey string, passphrase string) (*Cred, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cred := newCred()
 	cusername := C.CString(username)
 	defer C.free(unsafe.Pointer(cusername))
 	cpublickey := C.CString(publicKey)
@@ -76,19 +104,37 @@ func NewCredSshKeyFromMemory(username string, publicKey string, privateKey strin
 	cpassphrase := C.CString(passphrase)
 	defer C.free(unsafe.Pointer(cpassphrase))
 	ret := C.git_cred_ssh_key_memory_new(&cred.ptr, cusername, cpublickey, cprivatekey, cpassphrase)
-	return int(ret), cred
+	if ret != 0 {
+		cred.Free()
+		return nil, MakeGitError(ret)
+	}
+	return cred, nil
 }
 
-func NewCredSshKeyFromAgent(username string) (int, Cred) {
-	cred := Cred{}
+func NewCredSshKeyFromAgent(username string) (*Cred, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cred := newCred()
 	cusername := C.CString(username)
 	defer C.free(unsafe.Pointer(cusername))
 	ret := C.git_cred_ssh_key_from_agent(&cred.ptr, cusername)
-	return int(ret), cred
+	if ret != 0 {
+		cred.Free()
+		return nil, MakeGitError(ret)
+	}
+	return cred, nil
 }
 
-func NewCredDefault() (int, Cred) {
-	cred := Cred{}
+func NewCredDefault() (*Cred, error) {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	cred := newCred()
 	ret := C.git_cred_default_new(&cred.ptr)
-	return int(ret), cred
+	if ret != 0 {
+		cred.Free()
+		return nil, MakeGitError(ret)
+	}
+	return cred, nil
 }
diff --git a/remote.go b/remote.go
index 43ffd33..c966aab 100644
--- a/remote.go
+++ b/remote.go
@@ -51,7 +51,7 @@ const (
 
 type TransportMessageCallback func(str string) ErrorCode
 type CompletionCallback func(RemoteCompletion) ErrorCode
-type CredentialsCallback func(url string, username_from_url string, allowed_types CredType) (ErrorCode, *Cred)
+type CredentialsCallback func(url string, username_from_url string, allowed_types CredType) (*Cred, error)
 type TransferProgressCallback func(stats TransferProgress) ErrorCode
 type UpdateTipsCallback func(refname string, a *Oid, b *Oid) ErrorCode
 type CertificateCheckCallback func(cert *Certificate, valid bool, hostname string) ErrorCode
@@ -246,11 +246,17 @@ func credentialsCallback(_cred **C.git_cred, _url *C.char, _username_from_url *C
 	}
 	url := C.GoString(_url)
 	username_from_url := C.GoString(_username_from_url)
-	ret, cred := callbacks.CredentialsCallback(url, username_from_url, (CredType)(allowed_types))
+	cred, err := callbacks.CredentialsCallback(url, username_from_url, (CredType)(allowed_types))
+	if err != nil {
+		if gitError, ok := err.(*GitError); ok {
+			return int(gitError.Code)
+		}
+		return C.GIT_EUSER
+	}
 	if cred != nil {
 		*_cred = cred.ptr
 	}
-	return int(ret)
+	return 0
 }
 
 //export transferProgressCallback