code review 4022043: 8l: add PE dynexport

8l: add PE dynexport

[vcc, 2011-01-25 15:22:47]

Hello rsc, brainman (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[vcc, 2011-01-25 15:28:22]

for more test, download <
https://github.com/downloads/weigj/go-misc/callback.zip> , and run ./make.sh
&& ./hello
this example modify from swig/Examples/go/callback.

2011/1/25 <vcc.163@gmail.com>

> Reviewers: rsc, brainman,
>
> Message:
> Hello rsc, brainman (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> pe dynexport support.
>
> Please review this at http://codereview.appspot.com/4022043/
>
> Affected files:
>  M src/cmd/8l/asm.c
>  M src/cmd/ld/pe.c
>  M src/cmd/ld/pe.h
>
>
>

[rsc, 2011-01-25 15:33:40]

CL description should be 

8l: add PE dynexport

Looks fine to me; leaving for Alex.

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c
File src/cmd/ld/pe.c (right):

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode305
src/cmd/ld/pe.c:305: scmp(const void *p1,const void *p2 )
s/,/, /
s/ )/)/

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode311
src/cmd/ld/pe.c:311: return(strcmp(s1->dynimpname, s2->dynimpname));
drop () around return value

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode320
src/cmd/ld/pe.c:320: nexport= 0;
s/= 0/ = 0/

[vcc, 2011-01-25 15:58:18]

PTAL

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c
File src/cmd/ld/pe.c (right):

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode305
src/cmd/ld/pe.c:305: scmp(const void *p1,const void *p2 )
On 2011/01/25 15:33:40, rsc wrote:
> s/,/, /
> s/ )/)/

Done.

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode311
src/cmd/ld/pe.c:311: return(strcmp(s1->dynimpname, s2->dynimpname));
On 2011/01/25 15:33:40, rsc wrote:
> drop () around return value

Done.

http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode320
src/cmd/ld/pe.c:320: nexport= 0;
On 2011/01/25 15:33:40, rsc wrote:
> s/= 0/ = 0/

Done.

[mattn, 2011-01-27 09:00:22]

LGTM

BTW, It seems that local static variables in cgo section like following is not
exported.

----------------
/*
static int foo = 0;
*/
import "C"

func use_foo() {
  println(C.foo) // crash!
}
----------------

But I guess that this is an another issue.


On 2011/01/25 15:58:18, vcc wrote:
> PTAL
> 
> http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c
> File src/cmd/ld/pe.c (right):
> 
> http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode305
> src/cmd/ld/pe.c:305: scmp(const void *p1,const void *p2 )
> On 2011/01/25 15:33:40, rsc wrote:
> > s/,/, /
> > s/ )/)/
> 
> Done.
> 
> http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode311
> src/cmd/ld/pe.c:311: return(strcmp(s1->dynimpname, s2->dynimpname));
> On 2011/01/25 15:33:40, rsc wrote:
> > drop () around return value
> 
> Done.
> 
> http://codereview.appspot.com/4022043/diff/2001/src/cmd/ld/pe.c#newcode320
> src/cmd/ld/pe.c:320: nexport= 0;
> On 2011/01/25 15:33:40, rsc wrote:
> > s/= 0/ = 0/
> 
> Done.

[rsc, 2011-01-27 13:12:01]

LGTM

[rsc, 2011-01-27 13:26:33]

*** Submitted as http://code.google.com/p/go/source/detail?r=622140eb8862 ***

8l: add PE dynexport

R=rsc, brainman, mattn
CC=golang-dev
http://codereview.appspot.com/4022043

Committer: Russ Cox <rsc@golang.org>

[brainman, 2011-01-27 23:54:53]

LGTM