From b6703d47671b3a736e8b93ff0447da45e688865c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 18 Mar 2014 04:54:40 +0100 Subject: [PATCH 1/5] Oid: make the type directly [20]byte There is no need for a struct with a single field. An Oid is 20 bytes which hold the binary representation of the hash, so let's use that directly. Go lets us have methods on this new type just the same. --- git.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/git.go b/git.go index db9522b..8f543b5 100644 --- a/git.go +++ b/git.go @@ -29,9 +29,7 @@ func init() { } // Oid -type Oid struct { - bytes [20]byte -} +type Oid [20]byte func newOidFromC(coid *C.git_oid) *Oid { if coid == nil { @@ -39,18 +37,18 @@ func newOidFromC(coid *C.git_oid) *Oid { } oid := new(Oid) - copy(oid.bytes[0:20], C.GoBytes(unsafe.Pointer(coid), 20)) + copy(oid[0:20], C.GoBytes(unsafe.Pointer(coid), 20)) return oid } func NewOid(b []byte) *Oid { oid := new(Oid) - copy(oid.bytes[0:20], b[0:20]) + copy(oid[0:20], b[0:20]) return oid } func (oid *Oid) toC() *C.git_oid { - return (*C.git_oid)(unsafe.Pointer(&oid.bytes)) + return (*C.git_oid)(unsafe.Pointer(oid)) } func NewOidFromString(s string) (*Oid, error) { @@ -75,25 +73,25 @@ func (oid *Oid) String() string { } func (oid *Oid) Bytes() []byte { - return oid.bytes[0:] + return oid[0:] } func (oid *Oid) Cmp(oid2 *Oid) int { - return bytes.Compare(oid.bytes[:], oid2.bytes[:]) + return bytes.Compare(oid[:], oid2[:]) } func (oid *Oid) Copy() *Oid { ret := new(Oid) - copy(ret.bytes[:], oid.bytes[:]) + copy(ret[:], oid[:]) return ret } func (oid *Oid) Equal(oid2 *Oid) bool { - return bytes.Equal(oid.bytes[:], oid2.bytes[:]) + return bytes.Equal(oid[:], oid2[:]) } func (oid *Oid) IsZero() bool { - for _, a := range oid.bytes { + for _, a := range oid { if a != '0' { return false } @@ -102,7 +100,7 @@ func (oid *Oid) IsZero() bool { } func (oid *Oid) NCmp(oid2 *Oid, n uint) int { - return bytes.Compare(oid.bytes[:n], oid2.bytes[:n]) + return bytes.Compare(oid[:n], oid2[:n]) } func ShortenOids(ids []*Oid, minlen int) (int, error) { From c9c7c1e77942f88955af0dc3bdfb58d5e7d7f121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 18 Mar 2014 05:04:26 +0100 Subject: [PATCH 2/5] Oid: make NewOid take a string This is the most common way of having an id that's not in Oid form, so let's make it the "default" and rename to NewOidFromBytes() the one that takes []byte. --- git.go | 4 ++-- odb_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/git.go b/git.go index 8f543b5..f3fb7e1 100644 --- a/git.go +++ b/git.go @@ -41,7 +41,7 @@ func newOidFromC(coid *C.git_oid) *Oid { return oid } -func NewOid(b []byte) *Oid { +func NewOidFromBytes(b []byte) *Oid { oid := new(Oid) copy(oid[0:20], b[0:20]) return oid @@ -51,7 +51,7 @@ func (oid *Oid) toC() *C.git_oid { return (*C.git_oid)(unsafe.Pointer(oid)) } -func NewOidFromString(s string) (*Oid, error) { +func NewOid(s string) (*Oid, error) { o := new(Oid) cs := C.CString(s) defer C.free(unsafe.Pointer(cs)) diff --git a/odb_test.go b/odb_test.go index a4f8943..17b3ad2 100644 --- a/odb_test.go +++ b/odb_test.go @@ -27,7 +27,7 @@ func TestOdbStream(t *testing.T) { error = stream.Close() checkFatal(t, error) - expectedId, error := NewOidFromString("30f51a3fba5274d53522d0f19748456974647b4f") + expectedId, error := NewOid("30f51a3fba5274d53522d0f19748456974647b4f") checkFatal(t, error) if stream.Id.Cmp(expectedId) != 0 { t.Fatal("Wrong data written") @@ -59,4 +59,4 @@ Initial commit.`; if oid.Cmp(coid) != 0 { t.Fatal("Hash and write Oids are different") } -} \ No newline at end of file +} From c243c31f7d428680579a1dd20273cd3888c730e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 19 Mar 2014 03:11:41 +0100 Subject: [PATCH 3/5] Oid: remove Bytes() This is not needed. We can do id[:] to get a slice. --- git.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/git.go b/git.go index f3fb7e1..9e1d3e7 100644 --- a/git.go +++ b/git.go @@ -28,7 +28,7 @@ func init() { C.git_threads_init() } -// Oid +// Oid represents the id for a Git object. type Oid [20]byte func newOidFromC(coid *C.git_oid) *Oid { @@ -72,10 +72,6 @@ func (oid *Oid) String() string { return string(buf) } -func (oid *Oid) Bytes() []byte { - return oid[0:] -} - func (oid *Oid) Cmp(oid2 *Oid) int { return bytes.Compare(oid[:], oid2[:]) } From 0bb73e43a8f26be8608cdd304d73cacb05753417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 19 Mar 2014 03:39:34 +0100 Subject: [PATCH 4/5] Oid: use Go's conversion functions Go already has all the necessary pieces for encoding and decoding hex strings. Using them let's us avoid going into C land. Benchmarks show this takes about half the time as using libgit2's functions. --- git.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/git.go b/git.go index 9e1d3e7..e5fe812 100644 --- a/git.go +++ b/git.go @@ -8,6 +8,7 @@ package git import "C" import ( "bytes" + "encoding/hex" "errors" "runtime" "strings" @@ -52,24 +53,23 @@ func (oid *Oid) toC() *C.git_oid { } func NewOid(s string) (*Oid, error) { - o := new(Oid) - cs := C.CString(s) - defer C.free(unsafe.Pointer(cs)) - - runtime.LockOSThread() - defer runtime.UnlockOSThread() - - if ret := C.git_oid_fromstr(o.toC(), cs); ret < 0 { - return nil, MakeGitError(ret) + if len(s) > C.GIT_OID_HEXSZ { + return nil, errors.New("string is too long for oid") } + o := new(Oid) + + slice, error := hex.DecodeString(s) + if error != nil { + return nil, error + } + + copy(o[:], slice[:20]) return o, nil } func (oid *Oid) String() string { - buf := make([]byte, 40) - C.git_oid_fmt((*C.char)(unsafe.Pointer(&buf[0])), oid.toC()) - return string(buf) + return hex.EncodeToString(oid[:]) } func (oid *Oid) Cmp(oid2 *Oid) int { From b82a72a9ce4701a4560288c4ebf1511ffb415b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 19 Mar 2014 03:51:59 +0100 Subject: [PATCH 5/5] Oid: fix IsZero() We need to compare against the number zero, not its ASCII value. --- git.go | 2 +- git_test.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/git.go b/git.go index e5fe812..b20f993 100644 --- a/git.go +++ b/git.go @@ -88,7 +88,7 @@ func (oid *Oid) Equal(oid2 *Oid) bool { func (oid *Oid) IsZero() bool { for _, a := range oid { - if a != '0' { + if a != 0 { return false } } diff --git a/git_test.go b/git_test.go index fff3c6c..6542ca0 100644 --- a/git_test.go +++ b/git_test.go @@ -54,3 +54,11 @@ func seedTestRepo(t *testing.T, repo *Repository) (*Oid, *Oid) { return commitId, treeId } + +func TestOidZero(t *testing.T) { + var zeroId Oid + + if !zeroId.IsZero() { + t.Error("Zero Oid is not zero") + } +}