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

Issue 85240046: code review 85240046: cmd/nm: windows pe handling fixes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by brainman
Modified:
9 years, 11 months ago
Reviewers:
rsc
CC:
golang-codereviews, bradfitz, ruiu, rsc
Visibility:
Public.

Description

cmd/nm: windows pe handling fixes - output absolute addresses, not relative; - accept negative section numbers. Update issue 6936 Fixes issue 7738

Patch Set 1 #

Patch Set 2 : diff -r 63408464ba37 https://go.googlecode.com/hg/ #

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

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -2 lines) Patch
A src/cmd/nm/nm_test.go View 1 1 chunk +33 lines, -0 lines 2 comments Download
M src/cmd/nm/pe.go View 1 2 chunks +30 lines, -2 lines 4 comments Download

Messages

Total messages: 12
brainman
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
9 years, 11 months ago (2014-04-14 06:30:35 UTC) #1
brainman
I have used information from http://pierrelib.pagesperso-orange.fr/exec_formats/DJGPP_COFF.html. Alex
9 years, 11 months ago (2014-04-14 06:31:35 UTC) #2
bradfitz
R=rsc On Sun, Apr 13, 2014 at 11:31 PM, <alex.brainman@gmail.com> wrote: > I have used ...
9 years, 11 months ago (2014-04-14 20:07:47 UTC) #3
ruiu
https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go File src/cmd/nm/pe.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go#newcode44 src/cmd/nm/pe.go:44: sym.Code = 'T' // not sure about this I ...
9 years, 11 months ago (2014-04-17 00:06:57 UTC) #4
brainman
https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go File src/cmd/nm/pe.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go#newcode44 src/cmd/nm/pe.go:44: sym.Code = 'T' // not sure about this On ...
9 years, 11 months ago (2014-04-17 00:26:11 UTC) #5
ruiu
https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go File src/cmd/nm/pe.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go#newcode44 src/cmd/nm/pe.go:44: sym.Code = 'T' // not sure about this I ...
9 years, 11 months ago (2014-04-17 00:31:11 UTC) #6
brainman
On 2014/04/17 00:31:11, ruiu wrote: > I was looking at GNU binutil's nm man page. ...
9 years, 11 months ago (2014-04-17 00:35:24 UTC) #7
rsc
LGTM https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go File src/cmd/nm/pe.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/pe.go#newcode44 src/cmd/nm/pe.go:44: sym.Code = 'T' // not sure about this ...
9 years, 11 months ago (2014-04-17 02:10:40 UTC) #8
brainman
On 2014/04/17 02:10:40, rsc wrote: > ... > I'll change this to C for constant ...
9 years, 11 months ago (2014-04-17 02:14:24 UTC) #9
rsc
https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/nm_test.go File src/cmd/nm/nm_test.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/nm_test.go#newcode15 src/cmd/nm/nm_test.go:15: testfiles := []string{ I changed this to build a ...
9 years, 11 months ago (2014-04-17 02:14:42 UTC) #10
brainman
https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/nm_test.go File src/cmd/nm/nm_test.go (right): https://codereview.appspot.com/85240046/diff/40001/src/cmd/nm/nm_test.go#newcode15 src/cmd/nm/nm_test.go:15: testfiles := []string{ On 2014/04/17 02:14:43, rsc wrote: > ...
9 years, 11 months ago (2014-04-17 02:16:01 UTC) #11
rsc
9 years, 11 months ago (2014-04-17 02:17:44 UTC) #12
*** Submitted as https://code.google.com/p/go/source/detail?r=a6fc6a6b22ca ***

cmd/nm: windows pe handling fixes

- output absolute addresses, not relative;
- accept negative section numbers.

Update issue 6936
Fixes issue 7738

LGTM=rsc
R=golang-codereviews, bradfitz, ruiu, rsc
CC=golang-codereviews
https://codereview.appspot.com/85240046

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

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