code review 109430043: ogle: move symbol lookup into dwarf package

ogle: move symbol lookup into dwarf package
Also add a (trivial) test for PCToLine.
Also open up the frame data, as yet unused, in the elf and macho packages.

[r, 2014-07-02 19:40:14]

Hello nigeltao,

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

[nigeltao, 2014-07-07 05:05:34]

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/line.go
File debug/dwarf/line.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/line.go#newcode5
debug/dwarf/line.go:5: // Mapping from PC to lines.
Is this a file-level comment or a package-level (doc) comment?

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test.go
File debug/dwarf/pclntab_test.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:5: // Stripped-down, simplified version of
../../gosym/pclntab_test.go
I like to put file-level comments between the package clause and the imports.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:7: package dwarf_test
Is the "_test" necessary?

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:19: . "code.google.com/p/ogle/debug/dwarf"
I'd rather not do the dot import, and be explicit with "dwarf.Foo" below.

Or, drop the "_test" in "package dwarf_test"?

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:87: panic(err) // TODO
The TODO should be trivial if this function returned (*Data, error) and not just
*Data.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/symbol.go
File debug/dwarf/symbol.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/symbol.go#new...
debug/dwarf/symbol.go:5: // This file provides simple methods to access the
symbol table by name and address.
I like to put file-level comments between the package clause and the imports.

----
// Copyright etc.

package dwarf

// This file etc.

import "fmt"
----

[r, 2014-07-07 17:37:51]

Hello nigeltao@golang.org,

Please take another look.

[r, 2014-07-07 17:38:05]

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/line.go
File debug/dwarf/line.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/line.go#newcode5
debug/dwarf/line.go:5: // Mapping from PC to lines.
On 2014/07/07 05:05:33, nigeltao wrote:
> Is this a file-level comment or a package-level (doc) comment?

Done.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test.go
File debug/dwarf/pclntab_test.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:5: // Stripped-down, simplified version of
../../gosym/pclntab_test.go
On 2014/07/07 05:05:33, nigeltao wrote:
> I like to put file-level comments between the package clause and the imports.

Done, although I disagree with this policy.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:7: package dwarf_test
On 2014/07/07 05:05:33, nigeltao wrote:
> Is the "_test" necessary?

Yes. It imports elf, which imports dwarf.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:19: . "code.google.com/p/ogle/debug/dwarf"
On 2014/07/07 05:05:33, nigeltao wrote:
> I'd rather not do the dot import, and be explicit with "dwarf.Foo" below.
> 
> Or, drop the "_test" in "package dwarf_test"?

Stet.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:87: panic(err) // TODO
On 2014/07/07 05:05:33, nigeltao wrote:
> The TODO should be trivial if this function returned (*Data, error) and not
just
> *Data.

Done.

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/symbol.go
File debug/dwarf/symbol.go (right):

https://codereview.appspot.com/109430043/diff/30001/debug/dwarf/symbol.go#new...
debug/dwarf/symbol.go:5: // This file provides simple methods to access the
symbol table by name and address.
done, but that's a change from the origin of this source.

[nigeltao, 2014-07-08 00:05:39]

LGTM.

"go test code.google.com/p/ogle/debug/dwarf" works for me, on linux/amd64.

https://codereview.appspot.com/109430043/diff/50001/debug/dwarf/pclntab_test.go
File debug/dwarf/pclntab_test.go (right):

https://codereview.appspot.com/109430043/diff/50001/debug/dwarf/pclntab_test....
debug/dwarf/pclntab_test.go:78: func getData(file string, t *testing.T) (*Data,
error) {
Drop the *testing.T arg.

[r, 2014-07-08 16:37:54]

*** Submitted as https://code.google.com/p/ogle/source/detail?r=d4c9dc09c25f ***

ogle: move symbol lookup into dwarf package
Also add a (trivial) test for PCToLine.
Also open up the frame data, as yet unused, in the elf and macho packages.

LGTM=nigeltao
R=nigeltao
https://codereview.appspot.com/109430043