Scoped.h is deprecated in favour of the standardised UniquePtr. This change will remove use of Scoped.h everywhere in PSM except ScopedNSSTypes.h, which is exported. Other consumers of ScopedNSSTypes.h can move off Scoped.h at their own pace.

Scoped.h is deprecated in favour of the standardised UniquePtr. This patch removes use of Scoped.h everywhere in PSM except ScopedNSSTypes.h, which is exported. Other consumers of ScopedNSSTypes.h can move off Scoped.h at their own pace. This patch also changes parameters and return types of various functions to make ownership more explicit. Review commit: https://reviewboard.mozilla.org/r/54456/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54456/

https://reviewboard.mozilla.org/r/54456/#review51254 ::: security/certverifier/NSSCertDBTrustDomain.cpp:104 (Diff revision 1) > const SECItem encodedIssuerNameItem = { > siBuffer, > const_cast<unsigned char*>(encodedIssuerName.UnsafeGetData()), > encodedIssuerName.GetLength() > }; > - ScopedSECItem nameConstraints(::SECITEM_AllocItem(nullptr, nullptr, 0)); > + UniqueSECItem nameConstraints(::SECITEM_AllocItem(nullptr, nullptr, 0)); Actually, this probably makes more sense as a ScopedAutoSECItem.

Comment on attachment 8755243 [details] MozReview Request: Bug 1271496 - Stop using Scoped.h in non-exported PSM code. r=keeler https://reviewboard.mozilla.org/r/54456/#review51282 LGTM. ::: security/manager/ssl/SSLServerCertVerification.cpp:946 (Diff revision 1) > UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE)); > CERTGeneralName* subjectAltNames = > CERT_DecodeAltNameExtension(arena.get(), &altNameExtension); > // CERT_FindCertExtension takes a pointer to a SECItem and allocates memory > // in its data field. This is a bad way to do this because we can't use a > - // ScopedSECItem and neither is that memory tracked by an arena. We have to > + // UniqueSECItem and neither is that memory tracked by an arena. We have to I think this is what ScopedAutoSECItem is for, actually (as you pointed out in a similar situation, above). ::: security/manager/ssl/nsNSSU2FToken.cpp:114 (Diff revision 1) > if (aNickname == freeKeyName.get()) { > MOZ_LOG(gNSSTokenLog, LogLevel::Debug, ("Symmetric key found!")); > - return freeKey.forget(); > + return freeKey; > } > > keyList = PK11_GetNextSymKey(keyList); Unfortunately, it looks like we have a preexisting leak here - if it happens that we don't return the last element in the list, all remaining keys in the list will be leaked. This can be addressed in a follow-up, however. ::: security/manager/ssl/nsNSSU2FToken.cpp:128 (Diff revision 1) > - ScopedSECKEYPublicKey& aPubKey, const nsNSSShutDownPreventionLock&) > + /*out*/ UniqueSECKEYPrivateKey& aPrivKey, > + /*out*/ UniqueSECKEYPublicKey& aPubKey, > + const nsNSSShutDownPreventionLock&) > { > + MOZ_ASSERT(aSlot); > + NS_ENSURE_ARG_POINTER(aSlot); Well, aSlot isn't really a pointer, right? This might be best as |if (!aSlot) { return NS_ERROR_INVALID_ARG; }| ::: security/manager/ssl/nsNSSU2FToken.cpp:168 (Diff revision 1) > nsresult > nsNSSU2FToken::GetOrCreateWrappingKey(const UniquePK11SlotInfo& aSlot, > const nsNSSShutDownPreventionLock& locker) > { > + MOZ_ASSERT(aSlot); > + NS_ENSURE_ARG_POINTER(aSlot); Same here.

https://reviewboard.mozilla.org/r/54456/#review51282 Thanks! > I think this is what ScopedAutoSECItem is for, actually (as you pointed out in a similar situation, above). Indeed. I'll remove the comment and switch to using ScopedAutoSECItem in the relevant code. > Unfortunately, it looks like we have a preexisting leak here - if it happens that we don't return the last element in the list, all remaining keys in the list will be leaked. This can be addressed in a follow-up, however. Good point. I filed Bug 1275197. > Well, aSlot isn't really a pointer, right? This might be best as |if (!aSlot) { return NS_ERROR_INVALID_ARG; }| Will change as suggested. (I wish we could just use mozilla::NotNull being added in Bug 1272203, but it doesn't support UniquePtr. Oh well.)

Comment on attachment 8755243 [details] MozReview Request: Bug 1271496 - Stop using Scoped.h in non-exported PSM code. r=keeler Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54456/diff/1-2/

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d58a4ede1eea (The failures appear to be existing intermittents.)