code review 7817048: cmd/ld: permit sym to be both cgo_export_static and cgo...

cmd/ld: permit sym to be both cgo_export_static and cgo_export_dynamic

Fixes SWIG callbacks.  Previously crosscall2 was only
cgo_export_static, despite the use of two #pragma declarations
in runtime/cgo/callbacks.c.

[iant, 2013-03-23 00:56:01]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[iant, 2013-03-25 20:12:43]

Ping.

This change is pretty much obvious.  Can somebody who has looked at the linker
take a look at this?  Thanks.

[bradfitz, 2013-03-25 20:29:23]

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c
File src/cmd/ld/go.c (right):

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c#newcode525
src/cmd/ld/go.c:525: s->cgoexport |= b;
(Only reading this diff, and not really understanding whatever code this is)

all (two) non-return-error paths of this if statement do this line, so just move
it to be after the if instead and remove this else case?

and then why do you need the "b" variable?  you could just move the deleted
section of this patch to be after the if, no?

[rsc, 2013-03-25 22:06:28]

LGTM

Brad's cleanup sgtm too.

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c
File src/cmd/ld/go.c (right):

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c#newcode511
src/cmd/ld/go.c:511: b = CgoExportStatic;
tabs please.

[iant, 2013-03-25 22:37:40]

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c
File src/cmd/ld/go.c (right):

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c#newcode511
src/cmd/ld/go.c:511: b = CgoExportStatic;
On 2013/03/25 22:06:28, rsc wrote:
> tabs please.

Done.

https://codereview.appspot.com/7817048/diff/1/src/cmd/ld/go.c#newcode525
src/cmd/ld/go.c:525: s->cgoexport |= b;
On 2013/03/25 20:29:23, bradfitz wrote:
> (Only reading this diff, and not really understanding whatever code this is)
> 
> all (two) non-return-error paths of this if statement do this line, so just
move
> it to be after the if instead and remove this else case?
> 
> and then why do you need the "b" variable?  you could just move the deleted
> section of this patch to be after the if, no?

Good point, thanks.

[iant, 2013-03-25 22:37:59]

*** Submitted as https://code.google.com/p/go/source/detail?r=49bee4257d78 ***

cmd/ld: permit sym to be both cgo_export_static and cgo_export_dynamic

Fixes SWIG callbacks.  Previously crosscall2 was only
cgo_export_static, despite the use of two #pragma declarations
in runtime/cgo/callbacks.c.

R=golang-dev, bradfitz, rsc
CC=golang-dev
https://codereview.appspot.com/7817048