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

Issue 4603057: code review 4603057: runtime/cgo: fix for OS X 10.7 (Closed)

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

Description

runtime/cgo: fix for OS X 10.7 Correct a few error messages (libcgo -> runtime/cgo) and delete old nacl_386.c file too. Fixes issue 1657.

Patch Set 1 #

Patch Set 2 : diff -r 3a5f42c956e0 https://go.googlecode.com/hg #

Patch Set 3 : diff -r 3a5f42c956e0 https://go.googlecode.com/hg #

Total comments: 4

Patch Set 4 : diff -r dd3913e4b4b9 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -94 lines) Patch
M src/pkg/runtime/cgo/darwin_386.c View 1 2 3 3 chunks +36 lines, -36 lines 0 comments Download
M src/pkg/runtime/cgo/darwin_amd64.c View 1 2 3 2 chunks +27 lines, -38 lines 0 comments Download
R src/pkg/runtime/cgo/nacl_386.c View 1 1 chunk +0 lines, -19 lines 0 comments Download
M src/pkg/runtime/cgo/util.c View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6
rsc
Hello iant (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg
12 years, 9 months ago (2011-06-15 22:48:14 UTC) #1
iant
LGTM That is really special. http://codereview.appspot.com/4603057/diff/4001/src/pkg/runtime/cgo/darwin_386.c File src/pkg/runtime/cgo/darwin_386.c (right): http://codereview.appspot.com/4603057/diff/4001/src/pkg/runtime/cgo/darwin_386.c#newcode12 src/pkg/runtime/cgo/darwin_386.c:12: #define magic2 (0x12358132U) You ...
12 years, 9 months ago (2011-06-16 14:14:14 UTC) #2
rsc
> That is really special. Yes indeed. > http://codereview.appspot.com/4603057/diff/4001/src/pkg/runtime/cgo/darwin_386.c > File src/pkg/runtime/cgo/darwin_386.c (right): > > ...
12 years, 9 months ago (2011-06-16 14:40:53 UTC) #3
rsc
fwiw i took the volatiles out and the code broke. maybe the compiler reordered them ...
12 years, 9 months ago (2011-06-16 14:46:00 UTC) #4
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=0905a2ca94c6 *** runtime/cgo: fix for OS X 10.7 Correct a few error ...
12 years, 9 months ago (2011-06-16 15:10:49 UTC) #5
iant2
12 years, 9 months ago (2011-06-16 15:42:55 UTC) #6
Russ Cox <rsc@golang.org> writes:

> fwiw i took the volatiles out and the code broke.
> maybe the compiler reordered them above the pthread_setspecific?

Hmmm, yes, that is possible since they also depend on memory, but no
memory locations are listed as an input.  Sorry for missing that.  Might
as well keep them volatile.

Ian
Sign in to reply to this message.

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