From 7750e85fd1ff1006a28a5e292c9bc7ce3e12b586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 21 Apr 2015 13:14:22 +0200 Subject: [PATCH 01/15] Introduce an indirection layer for pointers As the Go runtime can move stacks at any point and the C code runs concurrently with the rest of the system, we cannot assume that the payloads we give to the C code will stay valid for any particular duration. We must therefore give the C code handles which we can then look up in our own list when the callbacks get called. --- git.go | 4 +++ handles.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 handles.go diff --git a/git.go b/git.go index 9496d2d..4f1a65e 100644 --- a/git.go +++ b/git.go @@ -93,7 +93,11 @@ var ( ErrInvalid = errors.New("Invalid state for operation") ) +var pointerHandles *HandleList + func init() { + pointerHandles = NewHandleList() + C.git_libgit2_init() // This is not something we should be doing, as we may be diff --git a/handles.go b/handles.go new file mode 100644 index 0000000..c0d1889 --- /dev/null +++ b/handles.go @@ -0,0 +1,82 @@ +package git + +import ( + "sync" + "unsafe" +) + +type HandleList struct { + sync.RWMutex + // stores the Go pointers + handles []interface{} + // indicates which indices are in use + set map[uintptr]bool +} + +func NewHandleList() *HandleList { + return &HandleList{ + handles: make([]interface{}, 5), + } +} + +// findUnusedSlot finds the smallest-index empty space in our +// list. You must only run this function while holding a write lock. +func (v *HandleList) findUnusedSlot() uintptr { + for i := 0; i < len(v.handles); i++ { + isUsed := v.set[uintptr(i)] + if !isUsed { + return uintptr(i) + } + } + + // reaching here means we've run out of entries so append and + // return the new index, which is equal to the old length. + slot := len(v.handles) + v.handles = append(v.handles, nil) + + return uintptr(slot) +} + +// Track adds the given pointer to the list of pointers to track and +// returns a pointer value which can be passed to C as an opaque +// pointer. +func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { + v.Lock() + + slot := v.findUnusedSlot() + v.handles[slot] = pointer + v.set[slot] = true + + v.Unlock() + + return unsafe.Pointer(slot) +} + +// Untrack stops tracking the pointer given by the handle +func (v *HandleList) Untrack(handle unsafe.Pointer) { + slot := uintptr(handle) + + v.Lock() + + v.handles[slot] = nil + delete(v.set, slot) + + v.Unlock() +} + +// Get retrieves the pointer from the given handle +func (v *HandleList) Get(handle unsafe.Pointer) interface{} { + slot := uintptr(handle) + + v.RLock() + + if _, ok := v.set[slot]; !ok { + panic("invalid pointer handle") + } + + ptr := v.handles[slot] + + v.RUnlock() + + return ptr +} From bde012f3d478752787bab0978e0bfaf5deefa42b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:02:57 +0200 Subject: [PATCH 02/15] handles: correctly initialize all members --- handles.go | 1 + 1 file changed, 1 insertion(+) diff --git a/handles.go b/handles.go index c0d1889..a862746 100644 --- a/handles.go +++ b/handles.go @@ -16,6 +16,7 @@ type HandleList struct { func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), + set: make(map[uintptr]bool), } } From be3a626f2ed67edd266bdf74e68adafcef1b0005 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:03:49 +0200 Subject: [PATCH 03/15] tree: use HandleList for C function callbacks. --- tree.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index c18d02a..70a3a3f 100644 --- a/tree.go +++ b/tree.go @@ -93,19 +93,25 @@ type TreeWalkCallback func(string, *TreeEntry) int func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { root := C.GoString((*C.char)(_root)) entry := (*C.git_tree_entry)(_entry) - callback := *(*TreeWalkCallback)(ptr) - return C.int(callback(root, newTreeEntry(entry))) + if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { + return C.int(callback(root, newTreeEntry(entry))) + } else { + return C.int(-1) + } } func (t Tree) Walk(callback TreeWalkCallback) error { runtime.LockOSThread() defer runtime.UnlockOSThread() + ptr := pointerHandles.Track(callback) + defer pointerHandles.Untrack(ptr) + err := C._go_git_treewalk( t.cast_ptr, C.GIT_TREEWALK_PRE, - unsafe.Pointer(&callback), + ptr, ) if err < 0 { From de45a4b8ed991cd46e64a1836e25dd9b182c821f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Apr 2015 10:33:00 +0200 Subject: [PATCH 04/15] submodule: use HandleList for C function callbacks --- submodule.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/submodule.go b/submodule.go index 7c6c922..0588ec5 100644 --- a/submodule.go +++ b/submodule.go @@ -97,17 +97,24 @@ func (repo *Repository) LookupSubmodule(name string) (*Submodule, error) { type SubmoduleCbk func(sub *Submodule, name string) int //export SubmoduleVisitor -func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, cfct unsafe.Pointer) C.int { +func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer) C.int { sub := &Submodule{(*C.git_submodule)(csub)} - fct := *(*SubmoduleCbk)(cfct) - return (C.int)(fct(sub, C.GoString(name))) + + if callback, ok := pointerHandles.Get(handle).(SubmoduleCbk); ok { + return (C.int)(callback(sub, C.GoString(name))) + } else { + return -1 + } } func (repo *Repository) ForeachSubmodule(cbk SubmoduleCbk) error { runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C._go_git_visit_submodule(repo.ptr, unsafe.Pointer(&cbk)) + handle := pointerHandles.Track(cbk) + defer pointerHandles.Untrack(handle) + + ret := C._go_git_visit_submodule(repo.ptr, handle) if ret < 0 { return MakeGitError(ret) } From 0a336e4abdd7c949c9dd38037165e6a16c7d7ebf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:35:58 +0200 Subject: [PATCH 05/15] handles: start slot indices with 1 Using 0 as the first slot indice leads to not being able to differentiate between a handle to the first element or a NULL-handle. As current code may check whether the pointer is NULL, change the first indice to be 1 instead. --- handles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handles.go b/handles.go index a862746..d173785 100644 --- a/handles.go +++ b/handles.go @@ -23,7 +23,7 @@ func NewHandleList() *HandleList { // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. func (v *HandleList) findUnusedSlot() uintptr { - for i := 0; i < len(v.handles); i++ { + for i := 1; i < len(v.handles); i++ { isUsed := v.set[uintptr(i)] if !isUsed { return uintptr(i) From 9bbec34885aff0287802134acbfdb5a20409fd9e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:40:49 +0200 Subject: [PATCH 06/15] index: use HandleList for C function callbacks. --- index.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/index.go b/index.go index 1a899f1..c344bf8 100644 --- a/index.go +++ b/index.go @@ -162,16 +162,17 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOpts, callback IndexMat runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_add_all( v.ptr, &cpathspecs, C.uint(flags), - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -188,15 +189,16 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback) runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_update_all( v.ptr, &cpathspecs, - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -213,15 +215,16 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) runtime.LockOSThread() defer runtime.UnlockOSThread() - var cb *IndexMatchedPathCallback + var handle unsafe.Pointer if callback != nil { - cb = &callback + handle = pointerHandles.Track(callback) + defer pointerHandles.Untrack(handle) } ret := C._go_git_index_remove_all( v.ptr, &cpathspecs, - unsafe.Pointer(cb), + handle, ) if ret < 0 { return MakeGitError(ret) @@ -231,8 +234,13 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) //export indexMatchedPathCallback func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int { - callback := (*IndexMatchedPathCallback)(payload) - return (*callback)(C.GoString(cPath), C.GoString(cMatchedPathspec)) + if payload == nil { + return 0 + } else if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { + return callback(C.GoString(cPath), C.GoString(cMatchedPathspec)) + } else { + return -1 + } } func (v *Index) RemoveByPath(path string) error { From e91965375551ff2ed68c0cea0c61a9ee4081ceb7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 09:55:06 +0200 Subject: [PATCH 07/15] odb: use HandleList for C function callbacks. --- odb.go | 13 ++++++++++--- odb_test.go | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/odb.go b/odb.go index ba03860..6b21329 100644 --- a/odb.go +++ b/odb.go @@ -98,8 +98,12 @@ type foreachData struct { } //export odbForEachCb -func odbForEachCb(id *C.git_oid, payload unsafe.Pointer) int { - data := (*foreachData)(payload) +func odbForEachCb(id *C.git_oid, handle unsafe.Pointer) int { + data, ok := pointerHandles.Get(handle).(*foreachData) + + if !ok { + panic("could not retrieve handle") + } err := data.callback(newOidFromC(id)) if err != nil { @@ -119,7 +123,10 @@ func (v *Odb) ForEach(callback OdbForEachCallback) error { runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&data)) + handle := pointerHandles.Track(&data) + defer pointerHandles.Untrack(handle) + + ret := C._go_git_odb_foreach(v.ptr, handle) if ret == C.GIT_EUSER { return data.err } else if ret < 0 { diff --git a/odb_test.go b/odb_test.go index 55ed297..2fb6840 100644 --- a/odb_test.go +++ b/odb_test.go @@ -81,7 +81,7 @@ func TestOdbForeach(t *testing.T) { checkFatal(t, err) if count != expect { - t.Fatalf("Expected %v objects, got %v") + t.Fatalf("Expected %v objects, got %v", expect, count) } expect = 1 From 7ee534d0c50f6b7b54a6e8c83e0953a95a4cac22 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:08:32 +0200 Subject: [PATCH 08/15] handles: print pointer handle on panic. --- handles.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/handles.go b/handles.go index d173785..1cbf6eb 100644 --- a/handles.go +++ b/handles.go @@ -1,6 +1,7 @@ package git import ( + "fmt" "sync" "unsafe" ) @@ -72,7 +73,7 @@ func (v *HandleList) Get(handle unsafe.Pointer) interface{} { v.RLock() if _, ok := v.set[slot]; !ok { - panic("invalid pointer handle") + panic(fmt.Sprintf("invalid pointer handle: %p", handle)) } ptr := v.handles[slot] From fe902f56a8545df76b51458b21aa84916ab51f6c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:10:09 +0200 Subject: [PATCH 09/15] diff: use HandleList for C function callbacks. --- diff.go | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/diff.go b/diff.go index 23469f2..8aa79aa 100644 --- a/diff.go +++ b/diff.go @@ -265,7 +265,11 @@ func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) err data := &diffForEachData{ FileCallback: cbFile, } - ecode := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, unsafe.Pointer(data)) + + handle := pointerHandles.Track(data) + defer pointerHandles.Untrack(handle) + + ecode := C._go_git_diff_foreach(diff.ptr, 1, intHunks, intLines, handle) if ecode < 0 { return data.Error } @@ -273,8 +277,12 @@ func (diff *Diff) ForEach(cbFile DiffForEachFileCallback, detail DiffDetail) err } //export diffForEachFileCb -func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe.Pointer) int { - data := (*diffForEachData)(payload) +func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } data.HunkCallback = nil if data.FileCallback != nil { @@ -292,8 +300,12 @@ func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe type DiffForEachHunkCallback func(DiffHunk) (DiffForEachLineCallback, error) //export diffForEachHunkCb -func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload unsafe.Pointer) int { - data := (*diffForEachData)(payload) +func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } data.LineCallback = nil if data.HunkCallback != nil { @@ -311,9 +323,12 @@ func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload u type DiffForEachLineCallback func(DiffLine) error //export diffForEachLineCb -func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, payload unsafe.Pointer) int { - - data := (*diffForEachData)(payload) +func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffForEachData) + if !ok { + panic("could not retrieve data for handle") + } err := data.LineCallback(diffLineFromC(delta, hunk, line)) if err != nil { @@ -479,9 +494,15 @@ type diffNotifyData struct { } //export diffNotifyCb -func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, payload unsafe.Pointer) int { +func diffNotifyCb(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_delta, matched_pathspec *C.char, handle unsafe.Pointer) int { diff_so_far := (*C.git_diff)(_diff_so_far) - data := (*diffNotifyData)(payload) + + payload := pointerHandles.Get(handle) + data, ok := payload.(*diffNotifyData) + if !ok { + panic("could not retrieve data for handle") + } + if data != nil { if data.Diff == nil { data.Diff = newDiffFromC(diff_so_far) @@ -507,6 +528,7 @@ func diffOptionsToC(opts *DiffOptions) (copts *C.git_diff_options, notifyData *d notifyData = &diffNotifyData{ Callback: opts.NotifyCallback, } + if opts.Pathspec != nil { cpathspec.count = C.size_t(len(opts.Pathspec)) cpathspec.strings = makeCStringsFromStrings(opts.Pathspec) @@ -527,7 +549,7 @@ func diffOptionsToC(opts *DiffOptions) (copts *C.git_diff_options, notifyData *d if opts.NotifyCallback != nil { C._go_git_setup_diff_notify_callbacks(copts) - copts.notify_payload = unsafe.Pointer(notifyData) + copts.notify_payload = pointerHandles.Track(notifyData) } } return @@ -539,6 +561,7 @@ func freeDiffOptions(copts *C.git_diff_options) { freeStrarray(&cpathspec) C.free(unsafe.Pointer(copts.old_prefix)) C.free(unsafe.Pointer(copts.new_prefix)) + pointerHandles.Untrack(copts.notify_payload) } } From 83f9e6a7053cae61cf835221b3a61bdeb2e528f4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:13:52 +0200 Subject: [PATCH 10/15] blob: use HandleList for C function callbacks. --- blob.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/blob.go b/blob.go index 5a33bd8..b1fc78a 100644 --- a/blob.go +++ b/blob.go @@ -60,8 +60,13 @@ type BlobCallbackData struct { } //export blobChunkCb -func blobChunkCb(buffer *C.char, maxLen C.size_t, payload unsafe.Pointer) int { - data := (*BlobCallbackData)(payload) +func blobChunkCb(buffer *C.char, maxLen C.size_t, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*BlobCallbackData) + if !ok { + panic("could not retrieve blob callback data") + } + goBuf, err := data.Callback(int(maxLen)) if err == io.EOF { return 0 @@ -83,8 +88,12 @@ func (repo *Repository) CreateBlobFromChunks(hintPath string, callback BlobChunk defer C.free(unsafe.Pointer(chintPath)) } oid := C.git_oid{} + payload := &BlobCallbackData{Callback: callback} - ecode := C._go_git_blob_create_fromchunks(&oid, repo.ptr, chintPath, unsafe.Pointer(payload)) + handle := pointerHandles.Track(payload) + defer pointerHandles.Untrack(handle) + + ecode := C._go_git_blob_create_fromchunks(&oid, repo.ptr, chintPath, handle) if payload.Error != nil { return nil, payload.Error } From a843b7247f24a2125c30c865f9d8ea706ce3d768 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 24 Apr 2015 10:16:12 +0200 Subject: [PATCH 11/15] packbuilder: use HandleList for C function callbacks. --- packbuilder.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packbuilder.go b/packbuilder.go index 54a8390..4dc352c 100644 --- a/packbuilder.go +++ b/packbuilder.go @@ -110,8 +110,13 @@ type packbuilderCbData struct { } //export packbuilderForEachCb -func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, payload unsafe.Pointer) int { - data := (*packbuilderCbData)(payload) +func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) int { + payload := pointerHandles.Get(handle) + data, ok := payload.(*packbuilderCbData) + if !ok { + panic("could not get packbuilder CB data") + } + slice := C.GoBytes(buf, C.int(size)) err := data.callback(slice) @@ -130,11 +135,13 @@ func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error { callback: callback, err: nil, } + handle := pointerHandles.Track(&data) + defer pointerHandles.Untrack(handle) runtime.LockOSThread() defer runtime.UnlockOSThread() - err := C._go_git_packbuilder_foreach(pb.ptr, unsafe.Pointer(&data)) + err := C._go_git_packbuilder_foreach(pb.ptr, handle) if err == C.GIT_EUSER { return data.err } From d95932c84a2b9c926490def345d71d45bb19f344 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 08:58:21 +0200 Subject: [PATCH 12/15] handles: panic when we cannot retrieve handle data --- index.go | 6 ++---- submodule.go | 2 +- tree.go | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/index.go b/index.go index c344bf8..c1bfb74 100644 --- a/index.go +++ b/index.go @@ -234,12 +234,10 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback) //export indexMatchedPathCallback func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int { - if payload == nil { - return 0 - } else if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { + if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok { return callback(C.GoString(cPath), C.GoString(cMatchedPathspec)) } else { - return -1 + panic("invalid matched path callback") } } diff --git a/submodule.go b/submodule.go index 0588ec5..3882462 100644 --- a/submodule.go +++ b/submodule.go @@ -103,7 +103,7 @@ func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer) if callback, ok := pointerHandles.Get(handle).(SubmoduleCbk); ok { return (C.int)(callback(sub, C.GoString(name))) } else { - return -1 + panic("invalid submodule visitor callback") } } diff --git a/tree.go b/tree.go index 70a3a3f..cbba08b 100644 --- a/tree.go +++ b/tree.go @@ -97,7 +97,7 @@ func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { return C.int(callback(root, newTreeEntry(entry))) } else { - return C.int(-1) + panic("invalid treewalk callback") } } From 1bd338af5e7a329c8ec5bd85500350795d0793d2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 09:50:16 +0200 Subject: [PATCH 13/15] handles: do not store handles by uintptr If we store values by uintptrs the GC may try to inspect their values when it kicks in. As the pointers are most likely invalid, this will result in an invalid pointer dereference, causing the program to panic. We can fix this by storing values by an int index value instead, returning pointers to those indices as handles instead. --- handles.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/handles.go b/handles.go index 1cbf6eb..ec62a48 100644 --- a/handles.go +++ b/handles.go @@ -11,23 +11,23 @@ type HandleList struct { // stores the Go pointers handles []interface{} // indicates which indices are in use - set map[uintptr]bool + set map[int]bool } func NewHandleList() *HandleList { return &HandleList{ handles: make([]interface{}, 5), - set: make(map[uintptr]bool), + set: make(map[int]bool), } } // findUnusedSlot finds the smallest-index empty space in our // list. You must only run this function while holding a write lock. -func (v *HandleList) findUnusedSlot() uintptr { +func (v *HandleList) findUnusedSlot() int { for i := 1; i < len(v.handles); i++ { - isUsed := v.set[uintptr(i)] + isUsed := v.set[i] if !isUsed { - return uintptr(i) + return i } } @@ -36,7 +36,7 @@ func (v *HandleList) findUnusedSlot() uintptr { slot := len(v.handles) v.handles = append(v.handles, nil) - return uintptr(slot) + return slot } // Track adds the given pointer to the list of pointers to track and @@ -51,12 +51,12 @@ func (v *HandleList) Track(pointer interface{}) unsafe.Pointer { v.Unlock() - return unsafe.Pointer(slot) + return unsafe.Pointer(&slot) } // Untrack stops tracking the pointer given by the handle func (v *HandleList) Untrack(handle unsafe.Pointer) { - slot := uintptr(handle) + slot := *(*int)(handle) v.Lock() @@ -68,7 +68,7 @@ func (v *HandleList) Untrack(handle unsafe.Pointer) { // Get retrieves the pointer from the given handle func (v *HandleList) Get(handle unsafe.Pointer) interface{} { - slot := uintptr(handle) + slot := *(*int)(handle) v.RLock() From c43afaf9c4f5abb7ded44d88c8e9e290f61362fd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 09:56:21 +0200 Subject: [PATCH 14/15] tree: use correct C callback signature --- tree.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index cbba08b..aad2c8d 100644 --- a/tree.go +++ b/tree.go @@ -90,8 +90,8 @@ func (t Tree) EntryCount() uint64 { type TreeWalkCallback func(string, *TreeEntry) int //export CallbackGitTreeWalk -func CallbackGitTreeWalk(_root unsafe.Pointer, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { - root := C.GoString((*C.char)(_root)) +func CallbackGitTreeWalk(_root *C.char, _entry unsafe.Pointer, ptr unsafe.Pointer) C.int { + root := C.GoString(_root) entry := (*C.git_tree_entry)(_entry) if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok { From e8531dd5c31fc87044e9061b18f37df9b05bd0ac Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 22 May 2015 10:01:50 +0200 Subject: [PATCH 15/15] diff: only untrack notify payload when it is set --- diff.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/diff.go b/diff.go index 8aa79aa..5e03175 100644 --- a/diff.go +++ b/diff.go @@ -561,7 +561,9 @@ func freeDiffOptions(copts *C.git_diff_options) { freeStrarray(&cpathspec) C.free(unsafe.Pointer(copts.old_prefix)) C.free(unsafe.Pointer(copts.new_prefix)) - pointerHandles.Untrack(copts.notify_payload) + if copts.notify_payload != nil { + pointerHandles.Untrack(copts.notify_payload) + } } }