Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(764)

Issue 8308009: Fix a potential CFRelease(NULL). (Closed)

Created:
8 years, 11 months ago by Nico
Modified:
8 years, 11 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Fix a potential CFRelease(NULL). It would happen if CreateClientCertificateChain() returns NULL. Found by the clang static analyzer. Probably doesn't happen in practice. BUG=none TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107041

Patch Set 1 #

Patch Set 2 : rebase #

Total comments: 1

Patch Set 3 : oh man #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -2 lines) Patch
M net/socket/ssl_client_socket_nss.cc View 1 2 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Nico
8 years, 11 months ago (2011-10-16 18:22:15 UTC) #1
Ryan Sleevi
LGTM, but you won't be able to CQ it - not a net/ OWNERS.
8 years, 11 months ago (2011-10-16 18:31:01 UTC) #2
Nico
eroman for OWNERS
8 years, 11 months ago (2011-10-16 18:57:32 UTC) #3
eroman
lgtm. oops didnt see this in my queue!
8 years, 11 months ago (2011-10-24 23:52:25 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/8308009/1
8 years, 11 months ago (2011-10-24 23:59:24 UTC) #5
commit-bot: I haz the power
Can't apply patch for file net/socket/ssl_client_socket_nss.cc. While running patch -p1 --forward --force; patching file net/socket/ssl_client_socket_nss.cc ...
8 years, 11 months ago (2011-10-24 23:59:25 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/8308009/6001
8 years, 11 months ago (2011-10-25 00:12:24 UTC) #7
Nico
Thanks for the review. After rebasing, it looks a bit different.
8 years, 11 months ago (2011-10-25 00:12:45 UTC) #8
Ryan Sleevi
http://codereview.chromium.org/8308009/diff/6001/net/socket/ssl_client_socket_nss.cc File net/socket/ssl_client_socket_nss.cc (right): http://codereview.chromium.org/8308009/diff/6001/net/socket/ssl_client_socket_nss.cc#newcode2388 net/socket/ssl_client_socket_nss.cc:2388: CFArrayGetCount(chain); nico: Unchecked commit box. Now, |cert_count| is no ...
8 years, 11 months ago (2011-10-25 00:14:16 UTC) #9
Nico
yes, done, thanks. i wonder if clang would've caught this…maybe CFGetArrayCount() is marked as pure?
8 years, 11 months ago (2011-10-25 00:16:02 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/8308009/9001
8 years, 11 months ago (2011-10-25 00:16:25 UTC) #11
commit-bot: I haz the power
8 years, 11 months ago (2011-10-25 02:05:49 UTC) #12
Change committed as 107041

Powered by Google App Engine
This is Rietveld 408576698