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.

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

net/http: connReader.handleReadError doesn't use its error parameter #24201

Closed
mvdan opened this issue Mar 1, 2018 · 4 comments
Closed

net/http: connReader.handleReadError doesn't use its error parameter #24201

mvdan opened this issue Mar 1, 2018 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@mvdan
Copy link
Member

mvdan commented Mar 1, 2018

This is the case since the func was added in October 2016, as it hasn't been touched since:

func (cr *connReader) handleReadError(err error) {
       cr.conn.cancelCtx()
       cr.closeNotify()
}

Should the func not receive the parameter, or make use of it?

I found this with a static analysis tool - I don't necessarily think this is a bug. I simply agree with the tool that the code is a bit suspicious.

/cc @bradfitz
@mvdan mvdan added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 1, 2018
@spf13
Copy link
Contributor

spf13 commented Mar 26, 2018

ping @bradfitz for decision

@andybons andybons added this to the Unplanned milestone Mar 26, 2018
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 11, 2018
@bradfitz bradfitz self-assigned this Jun 11, 2018
@bradfitz bradfitz modified the milestones: Unplanned, Go1.11 Jun 11, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 11, 2018
@gopherbot
Copy link

Change https://golang.org/cl/122675 mentions this issue: net/http: comment handleReadError more, superficially use its argument

@bradfitz
Copy link
Contributor

bradfitz commented Jul 9, 2018

@mvdan, @andybons and I were curious which static analysis tool found this, and whether it handles _ error okay now.

@mvdan
Copy link
Member Author

mvdan commented Jul 9, 2018

@bradfitz @andybons it's https://github.com/mvdan/unparam, which should be ok-ish at avoiding false positives. In particular, it does ignore unnamed and underscore parameters. But as always, if you find a bug, please report it :)

@golang golang locked and limited conversation to collaborators Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@bradfitz @spf13 @andybons @mvdan @gopherbot