Issue 293350043: Bug 1168425 - Reenable NSS_STRICT_SHUTDOWN and leak checking for gtests

Issue 293350043: Bug 1168425 - Reenable NSS_STRICT_SHUTDOWN and leak checking for gtests (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
7 years, 11 months ago by ttaubert

Modified:
7 years, 11 months ago

Reviewers:
ekr, mt, wtc1, ekr-rietveld

Visibility:
Public.

More Reviews

Description

Bug 1168425 - Reenable NSS_STRICT_SHUTDOWN and leak checking for gtests

Patch Set 1 #

Total comments: 4

Created: 7 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+13 lines, -7 lines)			Patch
M	external_tests/ssl_gtest/ssl_loopback_unittest.cc	View	2 chunks	+6 lines, -2 lines	0 comments	Download
M	lib/ssl/ssl3con.c	View	1 chunk	+3 lines, -0 lines	0 comments	Download
M	lib/ssl/sslsock.c	View	1 chunk	+1 line, -0 lines	4 comments	Download
M	lib/ssl/tls13con.c	View	3 chunks	+3 lines, -3 lines	0 comments	Download
M	tests/ssl_gtests/ssl_gtests.sh	View	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

ekr-rietveld

LGTM https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c File lib/ssl/sslsock.c (right): https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c#newcode3402 lib/ssl/sslsock.c:3402: PR_INIT_CLIST(&ss->ssl3.hs.cipherSpecs); Do we still need this is ssl3_InitState()

7 years, 11 months ago (2016-05-03 21:11:40 UTC) #1

wtc1

https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c File lib/ssl/sslsock.c (right): https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c#newcode3402 lib/ssl/sslsock.c:3402: PR_INIT_CLIST(&ss->ssl3.hs.cipherSpecs); On 2016/05/03 21:11:40, ekr-webrtc wrote: > Do we ...

7 years, 11 months ago (2016-05-23 17:22:56 UTC) #3

ttaubert

7 years, 11 months ago (2016-05-23 20:02:01 UTC) #4

7 years, 11 months ago (2016-05-23 20:22:43 UTC) #5

Message was sent while issue was closed.

https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c
File lib/ssl/sslsock.c (right):

https://codereview.appspot.com/293350043/diff/1/lib/ssl/sslsock.c#newcode3402
lib/ssl/sslsock.c:3402: PR_INIT_CLIST(&ss->ssl3.hs.cipherSpecs);
On 2016/05/23 20:02:00, ttaubert wrote:
> On 2016/05/23 17:22:56, wtc1 wrote:
> > Tim, could you answer Eric's question? It seems
> > that we may initialize these circular lists
> > redundantly in some cases, but it is safer to not
> > modify ssl3_InitState().
> 
> I'm not sure whether we still need this in ssl3_InitState() or not. I added it
> to ssl_NewSocket() based on |remoteKeyShares| which seems very similar and
> probably should be initialized and destroyed in the same spots.

In that case, I think that we can safely remove the redundant init for both
fields in ssl3_InitState().

Expand All Messages | Collapse All Messages