From 3c1ba8c40e4d654bfca8b535c861a63c41b16f27 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Sat, 27 Aug 2016 20:44:46 +0200 Subject: [PATCH 1/4] Add test for slice-to-slice and GCo pointer detection --- .travis.yml | 1 + blob_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/.travis.yml b/.travis.yml index f796389..016cf2d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,7 @@ go: - 1.3 - 1.4 - 1.5 + - 1.6 - tip matrix: diff --git a/blob_test.go b/blob_test.go index 2b5ec4f..652c50c 100644 --- a/blob_test.go +++ b/blob_test.go @@ -1,9 +1,21 @@ package git import ( + "bytes" "testing" ) +type bufWrapper struct { + buf [64]byte + pointer []byte +} + +func doublePointerBytes() []byte { + o := &bufWrapper{} + o.pointer = o.buf[0:10] + return o.pointer[0:1] +} + func TestCreateBlobFromBuffer(t *testing.T) { repo := createTestRepo(t) defer cleanupTestRepo(t, repo) @@ -14,4 +26,18 @@ func TestCreateBlobFromBuffer(t *testing.T) { if id.String() != "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391" { t.Fatal("Empty buffer did not deliver empty blob id") } + + for _, data := range []([]byte){[]byte("hello there"), doublePointerBytes()} { + expected := make([]byte, len(data)) + copy(expected, data) + id, err = repo.CreateBlobFromBuffer(data) + checkFatal(t, err) + + blob, err := repo.LookupBlob(id) + checkFatal(t, err) + if !bytes.Equal(blob.Contents(), expected) { + t.Fatal("Loaded bytes don't match original bytes:", + blob.Contents(), "!=", expected) + } + } } From b5d213c2c1229ea5de524ee24a9d9635a9cf303f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 27 Aug 2016 20:47:41 +0200 Subject: [PATCH 2/4] Remove unecessary copy --- blob_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/blob_test.go b/blob_test.go index 652c50c..6b7cd49 100644 --- a/blob_test.go +++ b/blob_test.go @@ -28,16 +28,14 @@ func TestCreateBlobFromBuffer(t *testing.T) { } for _, data := range []([]byte){[]byte("hello there"), doublePointerBytes()} { - expected := make([]byte, len(data)) - copy(expected, data) id, err = repo.CreateBlobFromBuffer(data) checkFatal(t, err) blob, err := repo.LookupBlob(id) checkFatal(t, err) - if !bytes.Equal(blob.Contents(), expected) { + if !bytes.Equal(blob.Contents(), data) { t.Fatal("Loaded bytes don't match original bytes:", - blob.Contents(), "!=", expected) + blob.Contents(), "!=", data) } } } From b41e4c4ac7c7ec4d45ec5d8903077bd01264549f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 27 Aug 2016 20:51:13 +0200 Subject: [PATCH 3/4] Work around Go 1.6's CGo pointer check It depends heavily on the expression at the call site an whether it can figure out whether we're using a slice or not, so provid an incantation that does this. --- blob.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/blob.go b/blob.go index 1a86e60..8b3e94f 100644 --- a/blob.go +++ b/blob.go @@ -37,15 +37,24 @@ func (repo *Repository) CreateBlobFromBuffer(data []byte) (*Oid, error) { defer runtime.UnlockOSThread() var id C.git_oid - var ptr unsafe.Pointer + var size C.size_t + // Go 1.6 added some increased checking of passing pointer to + // C, but its check depends on its expectations of waht we + // pass to the C function, so unless we take the address of + // its contents at the call site itself, it can fail when + // 'data' is a slice of a slice. + // + // When we're given an empty slice, create a dummy one where 0 + // isn't out of bounds. if len(data) > 0 { - ptr = unsafe.Pointer(&data[0]) + size = C.size_t(len(data)) } else { - ptr = unsafe.Pointer(nil) + data = []byte{0} + size = C.size_t(0) } - ecode := C.git_blob_create_frombuffer(&id, repo.ptr, ptr, C.size_t(len(data))) + ecode := C.git_blob_create_frombuffer(&id, repo.ptr, unsafe.Pointer(&data[0]), size) if ecode < 0 { return nil, MakeGitError(ecode) } From 5c678353faa4f180ee4ad8a5e58ca71e093cf757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 27 Aug 2016 20:52:07 +0200 Subject: [PATCH 4/4] Add Go 1.7 to the build list --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 016cf2d..79ad168 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,7 @@ go: - 1.4 - 1.5 - 1.6 + - 1.7 - tip matrix: