Wrap errors

Instead of locking the thread, what we can do is wrap each function in
such a way that we can get back an error object without fear that
switching threads once back in Go-land will make us lose the right
one.

config.go

@@ -4,6 +4,8 @@ package git
 #cgo pkg-config: libgit2
 #include <git2.h>
 #include <git2/errors.h>
+
+#include "wrap.h"
 */
 import "C"
 import (
@@ -14,56 +16,53 @@ type Config struct {
 	ptr *C.git_config
 }
 
-func (c *Config) LookupInt32(name string) (v int32, err error) {
+func (c *Config) LookupInt32(name string) (int32, error) {
 	var out C.int32_t
+	var err *C.git_error
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 
-	ret := C.git_config_get_int32(&out, c.ptr, cname)
+	if ret := C.e_git_config_get_int32(&out, c.ptr, cname, &err); ret < 0 {
-	if ret < 0 {
+		return 0, makeError(ret, err)
-		return 0, LastError()
 	}
 
 	return int32(out), nil
 }
 
-func (c *Config) LookupInt64(name string) (v int64, err error) {
+func (c *Config) LookupInt64(name string) (int64, error) {
 	var out C.int64_t
+	var err *C.git_error
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 
-	ret := C.git_config_get_int64(&out, c.ptr, cname)
+	if ret := C.e_git_config_get_int64(&out, c.ptr, cname, &err); ret < 0 {
-	if ret < 0 {
+		return 0, makeError(ret, err)
-		return 0, LastError()
 	}
 
 	return int64(out), nil
 }
 
-func (c *Config) LookupString(name string) (v string, err error) {
+func (c *Config) LookupString(name string) (string, error) {
 	var ptr *C.char
+	var err *C.git_error
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 
-	ret := C.git_config_get_string(&ptr, c.ptr, cname)
+	if ret := C.e_git_config_get_string(&ptr, c.ptr, cname, &err); ret < 0 {
-	if ret < 0 {
+		return "", makeError(ret, err)
-		return "", LastError()
 	}
 
 	return C.GoString(ptr), nil
 }
 
-func (c *Config) Set(name, value string) (err error) {
+func (c *Config) Set(name, value string) error {
 	cname := C.CString(name)
 	defer C.free(unsafe.Pointer(cname))
 
 	cvalue := C.CString(value)
 	defer C.free(unsafe.Pointer(cvalue))
 
-	ret := C.git_config_set_string(c.ptr, cname, cvalue)
+	var err *C.git_error
-	if ret < 0 {
+	ret := C.e_git_config_set_string(c.ptr, cname, cvalue, &err)
-		return LastError()
+	return makeError(ret, err)
-	}
-
-	return nil
 }

config_test.go

@@ -0,0 +1,28 @@
+package git
+
+import (
+	"os"
+	"testing"
+)
+
+func TestConfig(t *testing.T) {
+	repo := createTestRepo(t)
+	defer os.RemoveAll(repo.Workdir())
+
+	config, err := repo.Config()
+	checkFatal(t, err)
+
+	_, err = config.LookupInt32("core.repositoryformatversion")
+	checkFatal(t, err)
+	_, err = config.LookupString("this.doesnt.exist")
+	if err == nil {
+		t.Fatal("No error returned")
+	}
+	gitErr, ok := err.(*GitError)
+	if !ok {
+		t.Fatal("Bad error type")
+	}
+	if gitErr.Code != -3 {
+		t.Fatalf("Expected ENOTFOUND got %v\n", gitErr.Code)
+	}
+}

git.go

@@ -123,6 +123,7 @@ func ShortenOids(ids []*Oid, minlen int) (int, error) {
 
 type GitError struct {
 	Message string
+	Class   int
 	Code    int
 }
 
@@ -133,9 +134,26 @@ func (e GitError) Error() string{
 func LastError() error {
 	err := C.giterr_last()
 	if err == nil {
-		return &GitError{"No message", 0}
+		return &GitError{"No message", 0, -1}
 	}
-	return &GitError{C.GoString(err.message), int(err.klass)}
+	return &GitError{C.GoString(err.message), int(err.klass), -1}
+}
+
+// makeError creates a Go error out of the C error and frees its
+// string
+func makeError(code C.int, err *C.git_error) error {
+	if code == 0 {
+		return nil
+	}
+
+	error := &GitError{
+		Code:    int(code),
+		Class:   int(err.klass),
+		Message: C.GoString(err.message),
+	}
+
+	C.free(unsafe.Pointer(err.message))
+	return error
 }
 
 func cbool(b bool) C.int {

wrap.h

@@ -0,0 +1,40 @@
+#ifndef _GIT2GO_WRAP_H_
+#define _GIT2GO_WRAP_H_
+
+#include <string.h>
+
+static inline git_error *copy_error(void)
+{
+	git_error *err;
+	const git_error *last;
+
+	last = giterr_last();
+	if (last == NULL)
+		return NULL;
+
+	err = malloc(sizeof(git_error));
+	if (err == NULL)
+		return NULL;
+
+	err->klass = last->klass;
+	err->message = last->message ? strdup(last->message) : NULL;
+	return err;
+}
+
+#define WRAP(name, call)			\
+	int e_##name			\
+	{					\
+		int ret;			\
+		ret = call;			\
+		if (ret < 0)			\
+			*err = copy_error();	\
+		return ret;			\
+	}
+
+WRAP(git_config_get_string(const char **out, git_config *cfg, const char *name, git_error **err), git_config_get_string(out, cfg, name))
+WRAP(git_config_set_string(git_config *cfg, const char *name, const char *value, git_error **err), git_config_set_string(cfg, name, value))
+
+WRAP(git_config_get_int32(int32_t *out, git_config *cfg, const char *name, git_error **err), git_config_get_int32(out, cfg, name))
+WRAP(git_config_get_int64(int64_t *out, git_config *cfg, const char *name, git_error **err), git_config_get_int64(out, cfg, name))
+
+#endif /* _GIT2GO_WRAP_H_ */