Closed
Bug 1284272
Opened 8 years ago
Closed 8 years ago
Enable Chains tests on LSan runs
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.26
People
(Reporter: ttaubert, Assigned: ttaubert)
References
Details
Attachments
(2 files)
7.54 KB,
patch
|
franziskus
:
review+
|
Details | Diff | Splinter Review |
13.16 KB,
patch
|
franziskus
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
Hint: the referenceCount change doesn't really fix a leak since we also freed the CRL when referenceCount=-1, just wanted to fix it when I came across.
Attachment #8767696 -
Flags: review?(franziskuskiefer)
Comment 2•8 years ago
|
||
Comment on attachment 8767696 [details] [diff] [review] 0001-Bug-1284272-Enable-Chains-tests-on-LSan-runs.patch Review of attachment 8767696 [details] [diff] [review]: ----------------------------------------------------------------- lsan didn't complain about other paths in tstclnt? r+ with those fixed as well. ::: cmd/tstclnt/tstclnt.c @@ +1238,5 @@ > if (pingTimeoutSeconds >= 0) { > /* If caller requested a timeout, let's try just twice. */ > max_attempts = 2; > } > + PORT_Free(host); I see more returns in here that don't free host. We should probably change them to goto done.
Attachment #8767696 -
Flags: review?(franziskuskiefer) → review+
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #2) > lsan didn't complain about other paths in tstclnt? r+ with those fixed as > well. Yeah, if we don't cover a path LSan can't detect that. Coverity or scan-build could, maybe. > ::: cmd/tstclnt/tstclnt.c > @@ +1238,5 @@ > > if (pingTimeoutSeconds >= 0) { > > /* If caller requested a timeout, let's try just twice. */ > > max_attempts = 2; > > } > > + PORT_Free(host); > > I see more returns in here that don't free host. We should probably change > them to goto done. Good call, I replaced them with "goto done". Attached is a diff on top of the previous patch, I'll merge both.
Attachment #8768287 -
Flags: review?(franziskuskiefer)
Updated•8 years ago
|
Attachment #8768287 -
Flags: review?(franziskuskiefer) → review+
Assignee | ||
Comment 4•8 years ago
|
||
https://hg.mozilla.org/projects/nss/rev/a0ff2afa9316
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.26
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/projects/nss/rev/97a6fc5db60e
You need to log in
before you can comment on or make changes to this bug.
Description
•