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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 10 months ago by wtc
Modified:
2 years, 11 months ago
Reviewers:
eroman
CC:
chromium-reviews_googlegroups.com, darin, willchan
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) Lint Patch
M net/base/ssl_client_socket_win.cc View 1 4 chunks +16 lines, -7 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 4
wtc
4 years, 10 months ago #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 ...
4 years, 10 months ago #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 ...
4 years, 10 months ago #3
eroman
4 years, 10 months ago #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 1280:2d3e6564b7b6