Bug 1193021 - clean up reference-counting in security/; r=keeler
authorNathan Froyd <froydnj@mozilla.com>
Wed, 01 Jul 2015 13:10:53 -0400
changeset 257380 a369646b9ca5fa8ee90eb09122138149cbe9ed2f
parent 257379 69980d43dea7ef7a7dc333c7ae4cf7630e4ac289
child 257381 b4f66a22aba60a1c80cd5aba34ffe4a167daaaf0
push id14616
push userryanvm@gmail.com
push dateWed, 12 Aug 2015 14:50:53 +0000
treeherderfx-team@295bbb5db86b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1193021
milestone43.0a1
Bug 1193021 - clean up reference-counting in security/; r=keeler
security/manager/ssl/nsCertPicker.cpp
security/manager/ssl/nsNSSASN1Object.cpp
security/manager/ssl/nsNSSCertHelper.cpp
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsPK11TokenDB.cpp
security/manager/ssl/nsPKCS11Slot.cpp
--- a/security/manager/ssl/nsCertPicker.cpp
+++ b/security/manager/ssl/nsCertPicker.cpp
@@ -158,30 +158,22 @@ NS_IMETHODIMP nsCertPicker::PickByUsage(
   }
 
   if (NS_SUCCEEDED(rv) && !*canceled) {
     for (i = 0, node = CERT_LIST_HEAD(certList);
          !CERT_LIST_END(node, certList);
          ++i, node = CERT_LIST_NEXT(node)) {
 
       if (i == selectedIndex) {
-        nsNSSCertificate *cert = nsNSSCertificate::Create(node->cert);
+        nsRefPtr<nsNSSCertificate> cert = nsNSSCertificate::Create(node->cert);
         if (!cert) {
           rv = NS_ERROR_OUT_OF_MEMORY;
           break;
         }
 
-        nsIX509Cert *x509 = 0;
-        nsresult rv = cert->QueryInterface(NS_GET_IID(nsIX509Cert), (void**)&x509);
-        if (NS_FAILED(rv)) {
-          break;
-        }
-
-        NS_ADDREF(x509);
-        *_retval = x509;
-        NS_RELEASE(cert);
+        cert.forget(_retval);
         break;
       }
     }
   }
 
   return rv;
 }
--- a/security/manager/ssl/nsNSSASN1Object.cpp
+++ b/security/manager/ssl/nsNSSASN1Object.cpp
@@ -191,22 +191,21 @@ CreateFromDER(unsigned char *data,
 
   if (NS_SUCCEEDED(rv)) {
     // The actual object will be the first element inserted
     // into the sequence of the sequence variable we created.
     nsCOMPtr<nsIMutableArray> elements;
 
     sequence->GetASN1Objects(getter_AddRefs(elements));
     nsCOMPtr<nsIASN1Object> asn1Obj = do_QueryElementAt(elements, 0);
-    *retval = asn1Obj;
-    if (!*retval)
+    if (!asn1Obj) {
       return NS_ERROR_FAILURE;
+    }
 
-    NS_ADDREF(*retval);
-      
+    asn1Obj.forget(retval);
   }
   return rv; 
 }
 
 nsNSSASN1Sequence::nsNSSASN1Sequence() : mType(0),
                                          mTag(0),
                                          mIsValidContainer(true),
                                          mIsExpanded(true)
--- a/security/manager/ssl/nsNSSCertHelper.cpp
+++ b/security/manager/ssl/nsNSSCertHelper.cpp
@@ -125,18 +125,17 @@ ProcessVersion(SECItem         *versionI
     
   if (NS_FAILED(rv))
     return rv;
 
   rv = printableItem->SetDisplayValue(text);
   if (NS_FAILED(rv))
     return rv;
 
-  *retItem = printableItem;
-  NS_ADDREF(*retItem);
+  printableItem.forget(retItem);
   return NS_OK;
 }
 
 static nsresult 
 ProcessSerialNumberDER(SECItem         *serialItem, 
                        nsINSSComponent *nssComponent,
                        nsIASN1PrintableItem **retItem)
 {
@@ -153,18 +152,17 @@ ProcessSerialNumberDER(SECItem         *
     return rv;
 
   nsXPIDLCString serialNumber;
   serialNumber.Adopt(CERT_Hexify(serialItem, 1));
   if (!serialNumber)
     return NS_ERROR_OUT_OF_MEMORY;
 
   rv = printableItem->SetDisplayValue(NS_ConvertASCIItoUTF16(serialNumber));
-  *retItem = printableItem;
-  NS_ADDREF(*retItem);
+  printableItem.forget(retItem);
   return rv;
 }
 
 static nsresult
 GetDefaultOIDFormat(SECItem *oid,
                     nsINSSComponent *nssComponent,
                     nsAString &outString,
                     char separator)
@@ -1605,18 +1603,17 @@ ProcessSingleExtension(CERTCertExtension
                                      ev_oid_tag, nssComponent);
   if (NS_FAILED(rv)) {
     extvalue.Truncate();
     rv = ProcessRawBytes(nssComponent, &extension->value, extvalue, false);
   }
   text.Append(extvalue);
 
   extensionItem->SetDisplayValue(text);
-  *retExtension = extensionItem;
-  NS_ADDREF(*retExtension);
+  extensionItem.forget(retExtension);
   return NS_OK;
 }
 
 static nsresult
 ProcessSECAlgorithmID(SECAlgorithmID *algID,
                       nsINSSComponent *nssComponent,
                       nsIASN1Sequence **retSequence)
 {
@@ -1651,18 +1648,17 @@ ProcessSECAlgorithmID(SECAlgorithmID *al
        paramsOID.len = algID->parameters.len - 2;
        paramsOID.data = algID->parameters.data + 2;
        GetOIDText(&paramsOID, nssComponent, text);
     } else {
        ProcessRawBytes(nssComponent, &algID->parameters,text);
     }
     printableItem->SetDisplayValue(text);
   }
-  *retSequence = sequence;
-  NS_ADDREF(*retSequence);
+  sequence.forget(retSequence);
   return NS_OK;
 }
 
 static nsresult
 ProcessTime(PRTime dispTime, const char16_t *displayName, 
             nsIASN1Sequence *parentSequence)
 {
   nsresult rv;
@@ -2007,18 +2003,17 @@ nsNSSCertificate::CreateTBSCertificateAS
     if (!validEV)
       ev_oid_tag = SEC_OID_UNKNOWN;
 #endif
 
     rv = ProcessExtensions(mCert->extensions, sequence, ev_oid_tag, nssComponent);
     if (NS_FAILED(rv))
       return rv;
   }
-  *retSequence = sequence;
-  NS_ADDREF(*retSequence);  
+  sequence.forget(retSequence);
   return NS_OK;
 }
 
 nsresult
 nsNSSCertificate::CreateASN1Struct(nsIASN1Object** aRetVal)
 {
   static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
 
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -782,18 +782,17 @@ nsNSSCertificate::GetIssuer(nsIX509Cert*
   if (length == 1) { // No known issuer
     return NS_OK;
   }
   nsCOMPtr<nsIX509Cert> cert;
   chain->QueryElementAt(1, NS_GET_IID(nsIX509Cert), getter_AddRefs(cert));
   if (!cert) {
     return NS_ERROR_UNEXPECTED;
   }
-  *aIssuer = cert;
-  NS_ADDREF(*aIssuer);
+  cert.forget(aIssuer);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificate::GetOrganizationalUnit(nsAString& aOrganizationalUnit)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
@@ -1257,20 +1256,19 @@ nsNSSCertificate::GetCert()
 NS_IMETHODIMP
 nsNSSCertificate::GetValidity(nsIX509CertValidity** aValidity)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   NS_ENSURE_ARG(aValidity);
-  nsX509CertValidity* validity = new nsX509CertValidity(mCert.get());
+  nsRefPtr<nsX509CertValidity> validity = new nsX509CertValidity(mCert.get());
 
-  NS_ADDREF(validity);
-  *aValidity = static_cast<nsIX509CertValidity*>(validity);
+  validity.forget(aValidity);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificate::GetUsagesArray(bool localOnly,
                                  uint32_t* _verified,
                                  uint32_t* _count,
                                  char16_t*** _usages)
@@ -1718,18 +1716,17 @@ nsNSSCertList::GetEnumerator(nsISimpleEn
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
   nsCOMPtr<nsISimpleEnumerator> enumerator =
     new nsNSSCertListEnumerator(mCertList.get(), locker);
 
-  *_retval = enumerator;
-  NS_ADDREF(*_retval);
+  enumerator.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertList::Equals(nsIX509CertList* other, bool* result)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
@@ -1848,18 +1845,17 @@ nsNSSCertListEnumerator::GetNext(nsISupp
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIX509Cert> nssCert = nsNSSCertificate::Create(node->cert);
   if (!nssCert) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  *_retval = nssCert;
-  NS_ADDREF(*_retval);
+  nssCert.forget(_retval);
 
   CERT_RemoveCertListNode(node);
   return NS_OK;
 }
 
 // NB: This serialization must match that of nsNSSCertificateFakeTransport.
 NS_IMETHODIMP
 nsNSSCertificate::Write(nsIObjectOutputStream* aStream)
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -1380,22 +1380,21 @@ nsNSSCertificateDB::FindCertByEmailAddre
   }
 
   if (CERT_LIST_END(node, certlist)) {
     // no valid cert found
     return NS_ERROR_FAILURE;
   }
 
   // node now contains the first valid certificate with correct usage 
-  nsNSSCertificate *nssCert = nsNSSCertificate::Create(node->cert);
+  nsRefPtr<nsNSSCertificate> nssCert = nsNSSCertificate::Create(node->cert);
   if (!nssCert)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  NS_ADDREF(nssCert);
-  *_retval = static_cast<nsIX509Cert*>(nssCert);
+  nssCert.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::ConstructX509FromBase64(const char *base64,
                                             nsIX509Cert **_retval)
 {
   nsNSSShutDownPreventionLock locker;
@@ -1690,18 +1689,17 @@ nsNSSCertificateDB::GetCerts(nsIX509Cert
   nsCOMPtr<nsIInterfaceRequestor> ctx = new PipUIContext();
   nsCOMPtr<nsIX509CertList> nssCertList;
   ScopedCERTCertList certList(PK11_ListCerts(PK11CertListUnique, ctx));
 
   // nsNSSCertList 1) adopts certList, and 2) handles the nullptr case fine.
   // (returns an empty list) 
   nssCertList = new nsNSSCertList(certList, locker);
 
-  *_retval = nssCertList;
-  NS_ADDREF(*_retval);
+  nssCertList.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::VerifyCertNow(nsIX509Cert* aCert,
                                   int64_t /*SECCertificateUsage*/ aUsage,
                                   uint32_t aFlags,
                                   const char* aHostname,
--- a/security/manager/ssl/nsPK11TokenDB.cpp
+++ b/security/manager/ssl/nsPK11TokenDB.cpp
@@ -419,36 +419,36 @@ NS_IMETHODIMP nsPK11TokenDB::GetInternal
   nsresult rv = NS_OK;
   PK11SlotInfo *slot = 0;
   nsCOMPtr<nsIPK11Token> token;
 
   slot = PK11_GetInternalKeySlot();
   if (!slot) { rv = NS_ERROR_FAILURE; goto done; }
 
   token = new nsPK11Token(slot);
-  *_retval = token;
-  NS_ADDREF(*_retval);
+  token.forget(_retval);
 
 done:
   if (slot) PK11_FreeSlot(slot);
   return rv;
 }
 
 NS_IMETHODIMP nsPK11TokenDB::
 FindTokenByName(const char16_t* tokenName, nsIPK11Token **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   nsresult rv = NS_OK;
   PK11SlotInfo *slot = 0;
+  nsCOMPtr<nsIPK11Token> token;
   NS_ConvertUTF16toUTF8 aUtf8TokenName(tokenName);
   slot = PK11_FindSlotByName(const_cast<char*>(aUtf8TokenName.get()));
   if (!slot) { rv = NS_ERROR_FAILURE; goto done; }
 
-  *_retval = new nsPK11Token(slot);
-  NS_ADDREF(*_retval);
+  token = new nsPK11Token(slot);
+  token.forget(_retval);
 
 done:
   if (slot) PK11_FreeSlot(slot);
   return rv;
 }
 
 NS_IMETHODIMP nsPK11TokenDB::ListTokens(nsIEnumerator* *_retval)
 {
--- a/security/manager/ssl/nsPKCS11Slot.cpp
+++ b/security/manager/ssl/nsPKCS11Slot.cpp
@@ -164,18 +164,17 @@ nsPKCS11Slot::GetFWVersion(char16_t **aF
 NS_IMETHODIMP 
 nsPKCS11Slot::GetToken(nsIPK11Token **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   nsCOMPtr<nsIPK11Token> token = new nsPK11Token(mSlot);
-  *_retval = token;
-  NS_ADDREF(*_retval);
+  token.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsPKCS11Slot::GetTokenName(char16_t **aName)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
@@ -314,18 +313,17 @@ nsPKCS11Module::FindSlotByName(const cha
       // give up
       free(asciiname);
       return NS_ERROR_FAILURE;
     }
   } 
   free(asciiname);
   nsCOMPtr<nsIPKCS11Slot> slot = new nsPKCS11Slot(slotinfo);
   PK11_FreeSlot(slotinfo);
-  *_retval = slot;
-  NS_ADDREF(*_retval);
+  slot.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsPKCS11Module::ListSlots(nsIEnumerator **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
@@ -367,48 +365,45 @@ nsPKCS11ModuleDB::~nsPKCS11ModuleDB()
 NS_IMETHODIMP 
 nsPKCS11ModuleDB::GetInternal(nsIPKCS11Module **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   SECMODModule *nssMod = 
     SECMOD_CreateModule(nullptr, SECMOD_INT_NAME, nullptr, SECMOD_INT_FLAGS);
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(nssMod);
   SECMOD_DestroyModule(nssMod);
-  *_retval = module;
-  NS_ADDREF(*_retval);
+  module.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsPKCS11ModuleDB::GetInternalFIPS(nsIPKCS11Module **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   SECMODModule *nssMod = 
     SECMOD_CreateModule(nullptr, SECMOD_FIPS_NAME, nullptr, SECMOD_FIPS_FLAGS);
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(nssMod);
   SECMOD_DestroyModule(nssMod);
-  *_retval = module;
-  NS_ADDREF(*_retval);
+  module.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsPKCS11ModuleDB::FindModuleByName(const char16_t *aName,
                                    nsIPKCS11Module **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   NS_ConvertUTF16toUTF8 aUtf8Name(aName);
   SECMODModule *mod =
     SECMOD_FindModule(const_cast<char *>(aUtf8Name.get()));
   if (!mod)
     return NS_ERROR_FAILURE;
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(mod);
   SECMOD_DestroyModule(mod);
-  *_retval = module;
-  NS_ADDREF(*_retval);
+  module.forget(_retval);
   return NS_OK;
 }
 
 /* This is essentially the same as nsIPK11Token::findTokenByName, except
  * that it returns an nsIPKCS11Slot, which may be desired.
  */
 NS_IMETHODIMP 
 nsPKCS11ModuleDB::FindSlotByName(const char16_t *aName,
@@ -417,18 +412,17 @@ nsPKCS11ModuleDB::FindSlotByName(const c
   nsNSSShutDownPreventionLock locker;
   NS_ConvertUTF16toUTF8 aUtf8Name(aName);
   PK11SlotInfo *slotinfo =
    PK11_FindSlotByName(const_cast<char*>(aUtf8Name.get()));
   if (!slotinfo)
     return NS_ERROR_FAILURE;
   nsCOMPtr<nsIPKCS11Slot> slot = new nsPKCS11Slot(slotinfo);
   PK11_FreeSlot(slotinfo);
-  *_retval = slot;
-  NS_ADDREF(*_retval);
+  slot.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP 
 nsPKCS11ModuleDB::ListModules(nsIEnumerator **_retval)
 {
   nsNSSShutDownPreventionLock locker;
   nsresult rv = NS_OK;