From 48a02566d63adedb2dc7412996fb7ff5d6d26f72 Mon Sep 17 00:00:00 2001 From: lye Date: Sun, 23 Feb 2014 18:49:04 -0600 Subject: [PATCH] Pre-emptively copy data when marshalling diff callback data. When marshalling diff callback data to Go structs, any `char*` need to be pre-emptively copied onto Go's heap as they're invalidated as soon as our callback function returns. This patch adds this extra copy before sending the value to the channel, which fixes a bug wherein `DiffLine.Content`, `DiffFile.Path` and `DiffHunk.Header` would previously return garbage data. --- diff.go | 64 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/diff.go b/diff.go index 11493e1..0f3ee0f 100644 --- a/diff.go +++ b/diff.go @@ -40,16 +40,20 @@ const ( type DiffFile struct { file C.git_diff_file + Path string +} + +func newDiffFile(file *C.git_diff_file) *DiffFile { + return &DiffFile{ + file: *file, + Path: C.GoString(file.path), + } } func (df *DiffFile) Oid() *Oid { return newOidFromC(&df.file.oid) } -func (df *DiffFile) Path() string { - return C.GoString(df.file.path) -} - func (df *DiffFile) Size() int { return int(df.file.size) } @@ -64,6 +68,16 @@ func (df *DiffFile) Mode() uint16 { type DiffDelta struct { delta C.git_diff_delta + OldFile *DiffFile + NewFile *DiffFile +} + +func newDiffDelta(delta *C.git_diff_delta) *DiffDelta { + return &DiffDelta{ + delta: *delta, + OldFile: newDiffFile(&delta.old_file), + NewFile: newDiffFile(&delta.new_file), + } } func (dd *DiffDelta) Status() int { @@ -78,19 +92,20 @@ func (dd *DiffDelta) Similarity() uint16 { return uint16(dd.delta.similarity) } -func (dd *DiffDelta) OldFile() *DiffFile { - return &DiffFile{dd.delta.old_file} -} - -func (dd *DiffDelta) NewFile() *DiffFile { - return &DiffFile{dd.delta.new_file} -} - type DiffHunk struct { hunk C.git_diff_hunk + Header string DiffDelta } +func newDiffHunk(delta *C.git_diff_delta, hunk *C.git_diff_hunk) *DiffHunk { + return &DiffHunk{ + hunk: *hunk, + Header: C.GoStringN(&hunk.header[0], C.int(hunk.header_len)), + DiffDelta: *newDiffDelta(delta), + } +} + func (dh *DiffHunk) OldStart() int { return int(dh.hunk.old_start) } @@ -107,15 +122,20 @@ func (dh *DiffHunk) NewLines() int { return int(dh.hunk.new_lines) } -func (dh *DiffHunk) Header() string { - return C.GoStringN(&dh.hunk.header[0], C.int(dh.hunk.header_len)) -} - type DiffLine struct { line C.git_diff_line + Content string DiffHunk } +func newDiffLine(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.git_diff_line) *DiffLine { + return &DiffLine{ + line: *line, + Content: C.GoStringN(line.content, C.int(line.content_len)), + DiffHunk: *newDiffHunk(delta, hunk), + } +} + func (dl *DiffLine) Origin() byte { return byte(dl.line.origin) } @@ -132,10 +152,6 @@ func (dl *DiffLine) NumLines() int { return int(dl.line.num_lines) } -func (dl *DiffLine) Content() string { - return C.GoStringN(dl.line.content, C.int(dl.line.content_len)) -} - func (dl *DiffLine) ContentOffset() int { return int(dl.line.content_offset) } @@ -178,7 +194,7 @@ func diffForEachFileCb(delta *C.git_diff_delta, progress C.float, payload unsafe ch := *(*chan *DiffDelta)(payload) select { - case ch <-&DiffDelta{*delta}: + case ch <-newDiffDelta(delta): case <-ch: return -1 } @@ -202,7 +218,7 @@ func diffForEachHunkCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, payload u ch := *(*chan *DiffHunk)(payload) select { - case ch <-&DiffHunk{*hunk, DiffDelta{*delta}}: + case ch <-newDiffHunk(delta, hunk): case <-ch: return -1 } @@ -226,7 +242,7 @@ func diffForEachLineCb(delta *C.git_diff_delta, hunk *C.git_diff_hunk, line *C.g ch := *(*chan *DiffLine)(payload) select { - case ch <-&DiffLine{*line, DiffHunk{*hunk, DiffDelta{*delta}}}: + case ch <-newDiffLine(delta, hunk, line): case <-ch: return -1 } @@ -244,7 +260,7 @@ func (diff *Diff) GetDelta(index int) *DiffDelta { return nil } - return &DiffDelta{*ptr} + return newDiffDelta(ptr) } func (diff *Diff) Patch(deltaIndex int) *Patch {