Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(145)

Issue 834045: code review 834045: Library support for cgo export. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 5 months ago by iant
Modified:
9 years, 5 months ago
Reviewers:
CC:
rsc, golang-dev
Visibility:
Public.

Description

Library support for cgo export. These functions are used to call from a C function back to a Go function. This only includes 386 support.

Patch Set 1 #

Patch Set 2 : code review 834045: Library support for cgo export. #

Total comments: 10

Patch Set 3 : code review 834045: Library support for cgo export. #

Patch Set 4 : code review 834045: Library support for cgo export. #

Total comments: 1

Patch Set 5 : code review 834045: Library support for cgo export. #

Patch Set 6 : code review 834045: Library support for cgo export. #

Patch Set 7 : code review 834045: Library support for cgo export. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+140 lines, -5 lines) Patch
M src/libcgo/386.S View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
M src/pkg/runtime/386/asm.s View 1 2 3 4 1 chunk +38 lines, -1 line 0 comments Download
M src/pkg/runtime/cgocall.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M src/pkg/runtime/cgocall.c View 1 2 3 4 5 6 2 chunks +50 lines, -0 lines 0 comments Download
M src/pkg/runtime/proc.c View 1 2 3 4 5 6 2 chunks +24 lines, -2 lines 0 comments Download
M src/pkg/runtime/runtime.h View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 14
iant
Hello rsc (cc: golang-dev@googlegroups.com), I'd like you to review this change.
9 years, 5 months ago (2010-04-02 04:05:56 UTC) #1
rsc1
LGTM I'm glad you're writing this and not me! http://codereview.appspot.com/834045/diff/2001/3001 File src/libcgo/386.S (right): http://codereview.appspot.com/834045/diff/2001/3001#newcode39 src/libcgo/386.S:39: ...
9 years, 5 months ago (2010-04-02 06:31:25 UTC) #2
iant2
rsc@google.com writes: > http://codereview.appspot.com/834045/diff/2001/3002#newcode360 > src/pkg/runtime/386/asm.s:360: // runcgocallback(void* sp, void (*fn)()) > s/void* sp/void *sp/ ...
9 years, 5 months ago (2010-04-03 02:16:16 UTC) #3
rsc
LGTM
9 years, 5 months ago (2010-04-06 05:19:23 UTC) #4
rsc
Let's just use void (*fn)(void) everywhere. Most of the current instances of that really mean ...
9 years, 5 months ago (2010-04-06 05:19:28 UTC) #5
rsc1
On closer examination, --LGTM. I think there's something funny going on with the stack pointer ...
9 years, 5 months ago (2010-04-06 05:28:55 UTC) #6
iant2
rsc@google.com writes: > I think there's something funny going on with the stack > pointer ...
9 years, 5 months ago (2010-04-06 13:59:10 UTC) #7
rsc1
http://codereview.appspot.com/834045/diff/10002/18003 File src/pkg/runtime/cgocall.c (right): http://codereview.appspot.com/834045/diff/10002/18003#newcode48 src/pkg/runtime/cgocall.c:48: // When a C function calls back into Go, ...
9 years, 5 months ago (2010-04-06 21:11:50 UTC) #8
iant2
rsc@google.com writes: > http://codereview.appspot.com/834045/diff/10002/18003 > File src/pkg/runtime/cgocall.c (right): > > http://codereview.appspot.com/834045/diff/10002/18003#newcode48 > src/pkg/runtime/cgocall.c:48: // When ...
9 years, 5 months ago (2010-04-06 22:00:29 UTC) #9
rsc
> Hmmm. I suppose we could use > > #pragma textflag 7 > > but ...
9 years, 5 months ago (2010-04-06 23:33:09 UTC) #10
iant2
rsc@google.com writes: > Because this is a C function, the SP needs to match the ...
9 years, 5 months ago (2010-04-06 23:57:21 UTC) #11
iant2
Russ Cox <rsc@golang.org> writes: >> Hmmm. I suppose we could use >> >> #pragma textflag ...
9 years, 5 months ago (2010-04-07 00:02:09 UTC) #12
rsc1
LGTM
9 years, 5 months ago (2010-04-07 00:11:34 UTC) #13
iant
9 years, 5 months ago (2010-04-09 20:30:40 UTC) #14
*** Submitted as http://code.google.com/p/go/source/detail?r=cedf9f4db5c5 ***

Library support for cgo export.

These functions are used to call from a C function back to a
Go function.  This only includes 386 support.

R=rsc
CC=golang-dev
http://codereview.appspot.com/834045
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b