Issue 100500043: code review 100500043: sync/atomic: fix names in asm_amd64.s

Issue 100500043: code review 100500043: sync/atomic: fix names in asm_amd64.s (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 5 months ago by rsc

Modified:
12 years, 5 months ago

Reviewers:
r, gobot

CC:
r, bradfitz, golang-codereviews

Visibility:
Public.

Description

sync/atomic: fix unimportant assembly errors found by go vet None of these are real bugs. The variable name in the reference is not semantically meaningful, except that 'go vet' will double check the offset against the name for you. The stack sizes being corrected really are incorrect but they are also in NOSPLIT functions so they typically don't matter. Found by vet. GOOS=linux GOARCH=amd64 go vet sync/atomic GOOS=linux GOARCH=amd64p32 go vet sync/atomic GOOS=linux GOARCH=386 go vet sync/atomic GOOS=linux GOARCH=arm go vet sync/atomic GOOS=freebsd GOARCH=arm go vet sync/atomic GOOS=netbsd GOARCH=arm go vet sync/atomic

Patch Set 1 #

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

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

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

Total comments: 2

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

Created: 12 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+23 lines, -23 lines)			Patch
M	src/pkg/sync/atomic/asm_386.s	View	1 2 3	4 chunks	+7 lines, -7 lines	0 comments	Download
M	src/pkg/sync/atomic/asm_amd64.s	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	src/pkg/sync/atomic/asm_amd64p32.s	View	1 2 3	4 chunks	+7 lines, -7 lines	0 comments	Download
M	src/pkg/sync/atomic/asm_linux_arm.s	View	1 2 3	3 chunks	+6 lines, -6 lines	0 comments	Download
M	src/pkg/sync/atomic/export_linux_arm_test.go	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 8

Expand All Messages | Collapse All Messages

rsc

Hello r (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/

12 years, 5 months ago (2014-05-15 20:02:29 UTC) #1

bradfitz

https://codereview.appspot.com/100500043/diff/60001/src/pkg/sync/atomic/asm_amd64p32.s File src/pkg/sync/atomic/asm_amd64p32.s (right): https://codereview.appspot.com/100500043/diff/60001/src/pkg/sync/atomic/asm_amd64p32.s#newcode85 src/pkg/sync/atomic/asm_amd64p32.s:85: TEXT ·AddInt64(SB),NOSPLIT,$0-24 why isn't this 20, 4+8+8? I thought ...

12 years, 5 months ago (2014-05-15 20:24:20 UTC) #5

rsc

12 years, 5 months ago (2014-05-15 20:30:34 UTC) #6

rsc

*** Submitted as https://code.google.com/p/go/source/detail?r=100819a8fefa *** sync/atomic: fix unimportant assembly errors found by go vet None ...

12 years, 5 months ago (2014-05-15 20:31:22 UTC) #7

gobot

12 years, 5 months ago (2014-05-15 20:48:21 UTC) #8

Message was sent while issue was closed.

This CL appears to have broken the darwin-amd64-cheney builder.
See http://build.golang.org/log/9f43f74c246dfe3ec60f5db0f127de08f330aff7

Expand All Messages | Collapse All Messages