Closed
Bug 1255438
Opened 8 years ago
Closed 8 years ago
create nsI{Mutable,}Array directly in security/manager/ssl/
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(1 file)
7.44 KB,
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
The less involvement of the component manager, the better.
Attachment #8729018 -
Flags: review?(dkeeler)
Comment on attachment 8729018 [details] [diff] [review] create nsI{Mutable,}Array directly in security/manager/ssl/ Review of attachment 8729018 [details] [diff] [review]: ----------------------------------------------------------------- LGTM with comments addressed. ::: security/manager/pki/nsNSSDialogs.cpp @@ +32,5 @@ > #include "nsIX509CertValidity.h" > > #include "nsEmbedCID.h" > #include "nsIPromptService.h" > +#include "nsArray.h" If you would, please sort all of the #includes (we've been updating them to be in line with the style guidelines as we add/remove #includes in security/manager). ::: security/manager/ssl/nsCertTree.cpp @@ +14,5 @@ > #include "nsReadableUtils.h" > #include "nsUnicharUtils.h" > #include "nsNSSCertificate.h" > #include "nsNSSCertHelper.h" > +#include "nsArray.h" Same here. ::: security/manager/ssl/nsNSSASN1Object.cpp @@ +4,5 @@ > #include "nsNSSASN1Object.h" > #include "nsIComponentManager.h" > #include "secasn1.h" > #include "nsReadableUtils.h" > +#include "nsArray.h" ... etc. ::: security/manager/ssl/nsNSSCertificate.cpp @@ +892,1 @@ > if (NS_FAILED(rv)) { Looks like this should be: if (!array) { return NS_ERROR_FAILURE; } (also, looks like the done label can be removed) ::: security/manager/ssl/nsNSSCertificateDB.cpp @@ +445,5 @@ > if (!certCollection) { > PORT_FreeArena(arena, false); > return NS_ERROR_FAILURE; > } > + nsCOMPtr<nsIMutableArray> array = nsArrayBase::Create(); Note these changes may conflict with the patch in bug 1250258.
Attachment #8729018 -
Flags: review?(dkeeler) → review+
Comment 3•8 years ago
|
||
Comment on attachment 8729018 [details] [diff] [review] create nsI{Mutable,}Array directly in security/manager/ssl/ Review of attachment 8729018 [details] [diff] [review]: ----------------------------------------------------------------- FWIW, looks like there some unconverted lines such as https://hg.mozilla.org/mozilla-central/file/tip/security/manager/ssl/nsPK11TokenDB.cpp#l454. Not sure if they were just missed, or were omitted on purpose.
Comment 4•8 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #2) > Note these changes may conflict with the patch in bug 1250258. Yup. froydnj: Go ahead and land the changes here first if you like, the Bug 1250258 changes can wait.
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b125b74ee12b https://hg.mozilla.org/mozilla-central/rev/6dbd1b04414d
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•