Chromium Code Reviews
Help | Chromium Project | Sign in
(139)

Issue 141011: Don't put CredHandleClass in std::map directly because... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by wtc
Modified:
4 years ago
Reviewers:
eroman
CC:
chromium-reviews_googlegroups.com, darin (slow to review), willchan no longer on Chromium
Visibility:
Public.

Description

Don't put CredHandleClass in std::map directly because std::map may copy an entry to a new address while resizing, which invokes the destructor on the old entry and invalidates its address. R=eroman BUG=http://crbug.com/318 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=18879

Patch Set 1 #

Total comments: 3

Patch Set 2 : Adopt eroman's suggestion #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -7 lines) Patch
M net/base/ssl_client_socket_win.cc View 1 4 chunks +16 lines, -7 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 4 (0 generated)
wtc
5 years, 11 months ago (2009-06-19 23:07:14 UTC) #1
eroman
http://codereview.chromium.org/141011/diff/1/2 File net/base/ssl_client_socket_win.cc (right): http://codereview.chromium.org/141011/diff/1/2#newcode117 Line 117: handle = client_cert_creds_[key]; Have you tested this? This ...
5 years, 11 months ago (2009-06-19 23:27:31 UTC) #2
wtc
http://codereview.chromium.org/141011/diff/1/2 File net/base/ssl_client_socket_win.cc (right): http://codereview.chromium.org/141011/diff/1/2#newcode117 Line 117: handle = client_cert_creds_[key]; Yes, the fundamental data types ...
5 years, 11 months ago (2009-06-19 23:56:12 UTC) #3
eroman
5 years, 11 months ago (2009-06-19 23:57:52 UTC) #4
http://codereview.chromium.org/141011/diff/1/2
File net/base/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/141011/diff/1/2#newcode117
Line 117: handle = client_cert_creds_[key];
On 2009/06/19 23:56:12, wtc wrote:
> Yes, the fundamental data types are initialized with 0.
> 
> I'll use your version because this fact is obscure.  (I knew
> it because I wondered about it before and looked it up in a
> book.)

Ah, I stand corrected!
Thanks for looking that up.

LGTM as-is.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be