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

Issue 44080043: code review 44080043: net/http: use sync.Pool (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by bradfitz
Modified:
3 years, 9 months ago
Reviewers:
iant, dvyukov
CC:
golang-codereviews, iant2
Visibility:
Public.

Description

net/http: use sync.Pool Update Issue 4720

Patch Set 1 #

Patch Set 2 : diff -r 7f755d3040c0 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 7f755d3040c0 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r ff4cb9143edb https://go.googlecode.com/hg/ #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -49 lines) Patch
M src/pkg/net/http/header.go View 1 3 chunks +6 lines, -11 lines 2 comments Download
M src/pkg/net/http/request.go View 1 2 chunks +8 lines, -12 lines 0 comments Download
M src/pkg/net/http/server.go View 1 1 chunk +22 lines, -26 lines 0 comments Download

Messages

Total messages: 5
bradfitz
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
3 years, 9 months ago (2013-12-18 21:57:48 UTC) #1
iant
LGTM
3 years, 9 months ago (2013-12-18 22:14:50 UTC) #2
bradfitz
*** Submitted as https://code.google.com/p/go/source/detail?r=3d2ece96ab95 *** net/http: use sync.Pool Update Issue 4720 R=golang-dev, iant CC=golang-dev https://codereview.appspot.com/44080043
3 years, 9 months ago (2013-12-18 23:52:23 UTC) #3
dvyukov
https://codereview.appspot.com/44080043/diff/10002/src/pkg/net/http/header.go File src/pkg/net/http/header.go (right): https://codereview.appspot.com/44080043/diff/10002/src/pkg/net/http/header.go#newcode155 src/pkg/net/http/header.go:155: return err Don't we "leak" sorter here?
3 years, 9 months ago (2013-12-19 07:32:35 UTC) #4
bradfitz
3 years, 9 months ago (2013-12-26 20:37:24 UTC) #5
Message was sent while issue was closed.
https://codereview.appspot.com/44080043/diff/10002/src/pkg/net/http/header.go
File src/pkg/net/http/header.go (right):

https://codereview.appspot.com/44080043/diff/10002/src/pkg/net/http/header.go...
src/pkg/net/http/header.go:155: return err
On 2013/12/19 07:32:36, dvyukov wrote:
> Don't we "leak" sorter here?

Yes, but it doesn't matter, because it's an error path.  It was like that before
this CL too.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted