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

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

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

Comments

Projects
None yet
5 participants
@mvdan
Copy link
Member

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 label Mar 1, 2018

@spf13

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2018

ping @bradfitz for decision

@andybons andybons added this to the Unplanned milestone Mar 26, 2018

@bradfitz bradfitz added the NeedsFix label Jun 11, 2018

@bradfitz bradfitz self-assigned this Jun 11, 2018

@bradfitz bradfitz modified the milestones: Unplanned, Go1.11 Jun 11, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Jul 9, 2018

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

@gopherbot gopherbot closed this in 1da7f1e Jul 9, 2018

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 9, 2018

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

@mvdan

This comment has been minimized.

Copy link
Member Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.