Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(312)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 7047043: code review 7047043: cmd/cgo: allow for stdcall decorated dynimport names

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years ago by james4k
Modified:
12 years, 11 months ago
Reviewers:
rsc
CC:
minux1, adg, rsc, golang-dev
Visibility:
Public.
cmd/cgo: allow for stdcall decorated dynimport names To allow for stdcall decorated names on Windows, two changes were needed: 1. Change the symbol versioning delimiter '@' in cgo's dynimport output to a '#', and in cmd/ld when it parses dynimports. 2. Remove the "@N" decorator from the first argument of cgo's dynimport output (PE only). Fixes issue 4607.

Patch Set 1 #

Patch Set 2 : diff -r 21096a13f14b https://code.google.com/p/go #

Total comments: 2

Patch Set 3 : diff -r d0d76b7fb219 https://code.google.com/p/go #

Patch Set 4 : diff -r d0d76b7fb219 https://code.google.com/p/go #

Patch Set 5 : diff -r 49db9317937c https://code.google.com/p/go #

Created: 13 years ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M src/cmd/cgo/out.go View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M src/cmd/ld/go.c View 1 1 chunk +1 line, -1 line 0 comments Download
Total messages: 9
|
minux1
https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go File src/cmd/cgo/out.go (right): https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go#newcode221 src/cmd/cgo/out.go:221: name = name[:i] please consider using strings.Split(name, "@") then ...
13 years ago (2013年01月03日 10:49:43 UTC) #1
https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go
File src/cmd/cgo/out.go (right):
https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go#newcode221
src/cmd/cgo/out.go:221: name = name[:i]
please consider using strings.Split(name, "@")
then you don't need to do any conditionals here.
Sign in to reply to this message.
james4k
Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
13 years ago (2013年01月03日 20:52:24 UTC) #2
Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
https://code.google.com/p/go 
Sign in to reply to this message.
james4k
On 2013年01月03日 20:52:24, james4k wrote: > Hello mailto:minux.ma@gmail.com (cc: mailto:golang-dev@googlegroups.com), > > I'd like you ...
13 years ago (2013年01月03日 21:05:52 UTC) #3
On 2013年01月03日 20:52:24, james4k wrote:
> Hello mailto:minux.ma@gmail.com (cc: mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go
Hmm, I just uploaded a fix for an issue I didn't see at first (should split
ss[0], not s), but the side-by-side diff view doesn't seem to work anymore.
Sign in to reply to this message.
adg
Try hg sync hg upload 7047043
13 years ago (2013年01月06日 21:41:43 UTC) #4
Try
hg sync
hg upload 7047043
Sign in to reply to this message.
james4k
On 2013年01月06日 21:41:43, adg wrote: > Try > > hg sync > hg upload 7047043 ...
13 years ago (2013年01月07日 04:49:54 UTC) #5
On 2013年01月06日 21:41:43, adg wrote:
> Try
> 
> hg sync
> hg upload 7047043
Thanks adg, that worked.
Sign in to reply to this message.
james4k
PTAL. Do you think perhaps some sort of test would be necessary? Is there an ...
13 years ago (2013年01月07日 05:07:56 UTC) #6
PTAL. Do you think perhaps some sort of test would be necessary? Is there an
existing test for ELF symbol versioning with cgo?
https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go
File src/cmd/cgo/out.go (right):
https://codereview.appspot.com/7047043/diff/2001/src/cmd/cgo/out.go#newcode221
src/cmd/cgo/out.go:221: name = name[:i]
On 2013年01月03日 10:49:43, minux wrote:
> please consider using strings.Split(name, "@")
> then you don't need to do any conditionals here.
Done.
Sign in to reply to this message.
minux1
sorry for the long delay. please add a windows-only test to call stdcall decorated symbol ...
12 years, 11 months ago (2013年01月25日 11:17:50 UTC) #7
sorry for the long delay.
please add a windows-only test to call stdcall decorated symbol
to misc/cgo/test.
Sign in to reply to this message.
rsc
LGTM
12 years, 11 months ago (2013年01月30日 16:26:40 UTC) #8
LGTM
Sign in to reply to this message.
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=d8e64f78ab02 *** cmd/cgo: allow for stdcall decorated dynimport names To allow for ...
12 years, 11 months ago (2013年01月30日 16:29:38 UTC) #9
*** Submitted as https://code.google.com/p/go/source/detail?r=d8e64f78ab02 ***
cmd/cgo: allow for stdcall decorated dynimport names
To allow for stdcall decorated names on Windows, two changes were needed:
1. Change the symbol versioning delimiter '@' in cgo's dynimport output to a
'#', and in cmd/ld when it parses dynimports.
2. Remove the "@N" decorator from the first argument of cgo's dynimport output
(PE only).
Fixes issue 4607.
R=minux.ma, adg, rsc
CC=golang-dev
https://codereview.appspot.com/7047043
Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /