Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/cmd/gorename: panics when displaying diff #14009

Closed
ScottMansfield opened this issue Jan 19, 2016 · 7 comments
Closed

x/tools/cmd/gorename: panics when displaying diff #14009

ScottMansfield opened this issue Jan 19, 2016 · 7 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@ScottMansfield
Copy link

This is for the golang/tools repository, but can't open an issue there.

Running gorename with the diff display option panics when diff returns no data. Because err is not checked (because of the diff return codes) before display, the display part can panic when the diff output is nil.

My run on my local machine gave me a panic:

$ gorename -d -from '"github.com/netflix/rend/common".BAD_FLAGS' -to ErrorBadFlags
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x20 pc=0xa29b1]

goroutine 1 [running]:
golang.org/x/tools/refactor/rename.diff(0xc820898140, 0x46, 0xc823e76000, 0xe6b, 0xeb6, 0x0, 0x0)
    /Users/smansfield/gopkg/src/golang.org/x/tools/refactor/rename/rename.go:503 +0x3d1
golang.org/x/tools/refactor/rename.(*renamer).update(0xc823e41740, 0x0, 0x0)
    /Users/smansfield/gopkg/src/golang.org/x/tools/refactor/rename/rename.go:459 +0x10f7
golang.org/x/tools/refactor/rename.Main(0x48a700, 0x0, 0x0, 0x7fff5fbffa12, 0x2a, 0x7fff5fbffa41, 0xd, 0x0, 0x0)
    /Users/smansfield/gopkg/src/golang.org/x/tools/refactor/rename/rename.go:348 +0x1369
main.main()
    /Users/smansfield/gopkg/src/golang.org/x/tools/cmd/gorename/main.go:49 +0x337

return code: 2

I will be submitting a small patch to prevent this panic and provide some output.
@ScottMansfield ScottMansfield changed the title gorename panics when displaying diff and diff is nil gorename panics when displaying diff Jan 19, 2016
@ScottMansfield
Copy link
Author

After trying to patch locally, I noticed that stdout is actually nil. I may be missing something when running gorename, but I'm not sure what.

@ScottMansfield
Copy link
Author

Adding logic to set stdout to os.Stdout fixed the issue. The test used inside knowledge of the files to set the stdout variable to os.Stdout when running tests. To not break the tests, I've not restructured the files but added some logic to the flag parsing to set stdout properly. I'm working on getting gerrit set up to submit a patch.

@ianlancetaylor ianlancetaylor changed the title gorename panics when displaying diff x/tools/cmd/gorename: panics when displaying diff Jan 19, 2016
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jan 19, 2016
@ianlancetaylor
Copy link
Member

CC @alandonovan

@ScottMansfield
Copy link
Author

I have a change ready for review, but the CLA is taking a while to show up in gerrit. Just waiting on that before git codereview mail-ing

@ScottMansfield
Copy link
Author

Gerrit is not picking up the CLA I had already signed before signing up with Gerrit. Trying again doens't work, since I already signed once. Any chance of an assist here?

@bradfitz
Copy link
Contributor

@ScottMansfield, email me details (which email you're trying, whether it's your gmail, what Gerrit says, etc). My github username at golang.org.

@ScottMansfield
Copy link
Author

Submitted CL: https://go-review.googlesource.com/#/c/18813/

@golang golang locked and limited conversation to collaborators Jan 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @ScottMansfield @ianlancetaylor @gopherbot