Bug 1271501 - Remove unnecessary uses of reinterpret_cast in PSM. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Wed, 18 May 2016 18:58:40 -0700
changeset 298157 7503f7ee2446da1a859b8465d73a403289b93223
parent 298156 28176df6d571538bc28ec2ed88177ccd5da707ab
child 298158 4525aaa132ff6f642c9a4aaf8f0bb0ac6e5aa2d9
push id30273
push userkwierso@gmail.com
push dateFri, 20 May 2016 21:08:12 +0000
treeherdermozilla-central@c403ac05b8f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1271501
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1271501 - Remove unnecessary uses of reinterpret_cast in PSM. r=keeler These uses of reinterpret_cast are either pointless, or can be removed via refactoring. MozReview-Commit-ID: Aw2rlJfrT6J
security/certverifier/OCSPRequestor.cpp
security/manager/ssl/CertBlocklist.cpp
security/manager/ssl/nsCertTree.cpp
security/manager/ssl/nsIX509CertList.idl
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/nsNSSCallbacks.h
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSCertificateFakeTransport.cpp
security/manager/ssl/nsNTLMAuthModule.cpp
--- a/security/certverifier/OCSPRequestor.cpp
+++ b/security/certverifier/OCSPRequestor.cpp
@@ -135,24 +135,23 @@ DoOCSPRequest(const UniquePLArenaPool& a
     port = 80;
   } else if (port < 0 || port > 0xffff) {
     return Result::ERROR_CERT_BAD_ACCESS_LOCATION;
   }
   nsAutoCString
     hostname(url + authorityPos + hostnamePos,
              static_cast<nsACString_internal::size_type>(hostnameLen));
 
-  SEC_HTTP_SERVER_SESSION serverSessionPtr = nullptr;
+  nsNSSHttpServerSession* serverSessionPtr = nullptr;
   Result rv = nsNSSHttpInterface::createSessionFcn(
     hostname.BeginReading(), static_cast<uint16_t>(port), &serverSessionPtr);
   if (rv != Success) {
     return rv;
   }
-  UniqueHTTPServerSession serverSession(
-    reinterpret_cast<nsNSSHttpServerSession*>(serverSessionPtr));
+  UniqueHTTPServerSession serverSession(serverSessionPtr);
 
   nsAutoCString path;
   if (pathLen > 0) {
     path.Assign(url + pathPos, static_cast<nsAutoCString::size_type>(pathLen));
   } else {
     path.Assign("/");
   }
   MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
@@ -165,25 +164,24 @@ DoOCSPRequest(const UniquePLArenaPool& a
       path.Append("/");
     }
     nsresult nsrv = AppendEscapedBase64Item(encodedRequest, path);
     if (NS_WARN_IF(NS_FAILED(nsrv))) {
       return Result::FATAL_ERROR_LIBRARY_FAILURE;
     }
   }
 
-  SEC_HTTP_REQUEST_SESSION requestSessionPtr;
+  nsNSSHttpRequestSession* requestSessionPtr;
   rv = nsNSSHttpInterface::createFcn(serverSession.get(), "http", path.get(),
                                      method.get(), timeout, &requestSessionPtr);
   if (rv != Success) {
     return rv;
   }
 
-  UniqueHTTPRequestSession requestSession(
-    reinterpret_cast<nsNSSHttpRequestSession*>(requestSessionPtr));
+  UniqueHTTPRequestSession requestSession(requestSessionPtr);
 
   if (!useGET) {
     rv = nsNSSHttpInterface::setPostDataFcn(
       requestSession.get(), reinterpret_cast<char*>(encodedRequest->data),
       encodedRequest->len, "application/ocsp-request");
     if (rv != Success) {
       return rv;
     }
--- a/security/manager/ssl/CertBlocklist.cpp
+++ b/security/manager/ssl/CertBlocklist.cpp
@@ -580,18 +580,17 @@ CertBlocklist::IsCertRevoked(const uint8
   nsCOMPtr<nsICryptoHash> crypto;
   crypto = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
 
   rv = crypto->Init(nsICryptoHash::SHA256);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  rv = crypto->Update(reinterpret_cast<const unsigned char*>(aPubKey),
-                      aPubKeyLength);
+  rv = crypto->Update(aPubKey, aPubKeyLength);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   nsCString hashString;
   rv = crypto->Finish(false, hashString);
   if (NS_FAILED(rv)) {
     return rv;
--- a/security/manager/ssl/nsCertTree.cpp
+++ b/security/manager/ssl/nsCertTree.cpp
@@ -629,17 +629,17 @@ nsCertTree::GetCertsByTypeFromCache(nsIX
   // easily. We still have to acquire a shutdown prevention lock to prevent NSS
   // shutting down after GetRawCertList has returned. While cumbersome, this is
   // at least mostly correct. The rest of this implementation doesn't even go
   // this far in attempting to check for or prevent NSS shutdown at the
   // appropriate times. If this were reimplemented at a higher level using
   // more encapsulated types that handled NSS shutdown themselves, we wouldn't
   // be having these kinds of problems.
   nsNSSShutDownPreventionLock locker;
-  CERTCertList *certList = reinterpret_cast<CERTCertList*>(aCache->GetRawCertList());
+  CERTCertList* certList = aCache->GetRawCertList();
   if (!certList)
     return NS_ERROR_FAILURE;
   return GetCertsByTypeFromCertList(certList, aType, aCertCmpFn, aCertCmpFnArg);
 }
 
 // LoadCerts
 //
 // Load all of the certificates in the DB for this type.  Sort them
--- a/security/manager/ssl/nsIX509CertList.idl
+++ b/security/manager/ssl/nsIX509CertList.idl
@@ -2,26 +2,33 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 interface nsISimpleEnumerator;
 interface nsIX509Cert;
 
+%{C++
+typedef struct CERTCertListStr CERTCertList;
+%}
+[ptr] native CERTCertListPtr(CERTCertList);
+
 [scriptable, uuid(ae74cda5-cd2f-473f-96f5-f0b7fff62c68)]
 interface nsIX509CertList : nsISupports {
    void addCert(in nsIX509Cert cert);
    void deleteCert(in nsIX509Cert cert);
    nsISimpleEnumerator getEnumerator();
 
-   /* getRawCertList MUST be called only from functions where
-    * the nssShutdownPreventionLock has been adquired.
+   /**
+    * Returns the raw, backing cert list.
+    * Must be called only from functions where an nsNSSShutDownPreventionLock
+    * has been acquired.
     */
-   [notxpcom, noscript] voidPtr getRawCertList();
+   [notxpcom, noscript] CERTCertListPtr getRawCertList();
 
    /**
     * Test whether two certificate list instances represent the same
     * certificate list.
     *
     * @return Whether the certificate lists are equal
     */
    boolean equals(in nsIX509CertList other);
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -183,17 +183,17 @@ struct nsCancelHTTPDownloadEvent : Runna
     mListener = nullptr;
     return NS_OK;
   }
 };
 
 Result
 nsNSSHttpServerSession::createSessionFcn(const char* host,
                                          uint16_t portnum,
-                                         SEC_HTTP_SERVER_SESSION* pSession)
+                                 /*out*/ nsNSSHttpServerSession** pSession)
 {
   if (!host || !pSession) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
 
   nsNSSHttpServerSession* hss = new nsNSSHttpServerSession;
   if (!hss) {
     return Result::FATAL_ERROR_NO_MEMORY;
@@ -202,57 +202,52 @@ nsNSSHttpServerSession::createSessionFcn
   hss->mHost = host;
   hss->mPort = portnum;
 
   *pSession = hss;
   return Success;
 }
 
 Result
-nsNSSHttpRequestSession::createFcn(SEC_HTTP_SERVER_SESSION session,
+nsNSSHttpRequestSession::createFcn(const nsNSSHttpServerSession* session,
                                    const char* http_protocol_variant,
                                    const char* path_and_query_string,
                                    const char* http_request_method,
                                    const PRIntervalTime timeout,
-                                   SEC_HTTP_REQUEST_SESSION* pRequest)
+                           /*out*/ nsNSSHttpRequestSession** pRequest)
 {
   if (!session || !http_protocol_variant || !path_and_query_string ||
       !http_request_method || !pRequest) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
 
-  nsNSSHttpServerSession* hss = static_cast<nsNSSHttpServerSession*>(session);
-  if (!hss) {
-    return Result::FATAL_ERROR_INVALID_ARGS;
-  }
-
   nsNSSHttpRequestSession* rs = new nsNSSHttpRequestSession;
   if (!rs) {
     return Result::FATAL_ERROR_NO_MEMORY;
   }
 
   rs->mTimeoutInterval = timeout;
 
   // Use a maximum timeout value of 10 seconds because of bug 404059.
   // FIXME: Use a better approach once 406120 is ready.
   uint32_t maxBug404059Timeout = PR_TicksPerSecond() * 10;
   if (timeout > maxBug404059Timeout) {
     rs->mTimeoutInterval = maxBug404059Timeout;
   }
 
   rs->mURL.Assign(http_protocol_variant);
   rs->mURL.AppendLiteral("://");
-  rs->mURL.Append(hss->mHost);
+  rs->mURL.Append(session->mHost);
   rs->mURL.Append(':');
-  rs->mURL.AppendInt(hss->mPort);
+  rs->mURL.AppendInt(session->mPort);
   rs->mURL.Append(path_and_query_string);
 
   rs->mRequestMethod = http_request_method;
 
-  *pRequest = (void*)rs;
+  *pRequest = rs;
   return Success;
 }
 
 Result
 nsNSSHttpRequestSession::setPostDataFcn(const char* http_data,
                                         const uint32_t http_data_len,
                                         const char* http_content_type)
 {
--- a/security/manager/ssl/nsNSSCallbacks.h
+++ b/security/manager/ssl/nsNSSCallbacks.h
@@ -79,33 +79,33 @@ class nsNSSHttpServerSession
 public:
   typedef mozilla::pkix::Result Result;
 
   nsCString mHost;
   uint16_t mPort;
 
   static Result createSessionFcn(const char* host,
                                  uint16_t portnum,
-                                 SEC_HTTP_SERVER_SESSION* pSession);
+                         /*out*/ nsNSSHttpServerSession** pSession);
 };
 
 class nsNSSHttpRequestSession
 {
 protected:
   mozilla::ThreadSafeAutoRefCnt mRefCount;
 
 public:
   typedef mozilla::pkix::Result Result;
 
-  static Result createFcn(SEC_HTTP_SERVER_SESSION session,
+  static Result createFcn(const nsNSSHttpServerSession* session,
                           const char* httpProtocolVariant,
                           const char* pathAndQueryString,
                           const char* httpRequestMethod,
                           const PRIntervalTime timeout,
-                          SEC_HTTP_REQUEST_SESSION* pRequest);
+                  /*out*/ nsNSSHttpRequestSession** pRequest);
 
   Result setPostDataFcn(const char* httpData,
                         const uint32_t httpDataLen,
                         const char* httpContentType);
 
   Result trySendAndReceiveFcn(PRPollDesc** pPollDesc,
                               uint16_t* httpResponseCode,
                               const char** httpResponseContentType,
@@ -142,51 +142,50 @@ protected:
 
 class nsNSSHttpInterface
 {
 public:
   typedef mozilla::pkix::Result Result;
 
   static Result createSessionFcn(const char* host,
                                  uint16_t portnum,
-                                 SEC_HTTP_SERVER_SESSION* pSession)
+                         /*out*/ nsNSSHttpServerSession** pSession)
   {
     return nsNSSHttpServerSession::createSessionFcn(host, portnum, pSession);
   }
 
-  static Result createFcn(SEC_HTTP_SERVER_SESSION session,
+  static Result createFcn(const nsNSSHttpServerSession* session,
                           const char* httpProtocolVariant,
                           const char* pathAndQueryString,
                           const char* httpRequestMethod,
                           const PRIntervalTime timeout,
-                          SEC_HTTP_REQUEST_SESSION* pRequest)
+                  /*out*/ nsNSSHttpRequestSession** pRequest)
   {
     return nsNSSHttpRequestSession::createFcn(session, httpProtocolVariant,
                                               pathAndQueryString,
                                               httpRequestMethod, timeout,
                                               pRequest);
   }
 
-  static Result setPostDataFcn(SEC_HTTP_REQUEST_SESSION request,
+  static Result setPostDataFcn(nsNSSHttpRequestSession* request,
                                const char* httpData,
                                const uint32_t httpDataLen,
                                const char* httpContentType)
   {
-    return static_cast<nsNSSHttpRequestSession*>(request)
-      ->setPostDataFcn(httpData, httpDataLen, httpContentType);
+    return request->setPostDataFcn(httpData, httpDataLen, httpContentType);
   }
 
-  static Result trySendAndReceiveFcn(SEC_HTTP_REQUEST_SESSION request,
+  static Result trySendAndReceiveFcn(nsNSSHttpRequestSession* request,
                                      PRPollDesc** pPollDesc,
                                      uint16_t* httpResponseCode,
                                      const char** httpResponseContentType,
                                      const char** httpResponseHeaders,
                                      const char** httpResponseData,
                                      uint32_t* httpResponseDataLen)
   {
-    return static_cast<nsNSSHttpRequestSession*>(request)
-      ->trySendAndReceiveFcn(pPollDesc, httpResponseCode,
-                             httpResponseContentType, httpResponseHeaders,
-                             httpResponseData, httpResponseDataLen);
+    return request->trySendAndReceiveFcn(pPollDesc, httpResponseCode,
+                                         httpResponseContentType,
+                                         httpResponseHeaders,
+                                         httpResponseData, httpResponseDataLen);
   }
 };
 
 #endif // nsNSSCallbacks_h
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -1629,17 +1629,17 @@ nsNSSCertList::DupCertList(const UniqueC
       return nullptr;
     }
 
     Unused << cert.release(); // Ownership transferred to the cert list.
   }
   return newList;
 }
 
-void*
+CERTCertList*
 nsNSSCertList::GetRawCertList()
 {
   // This function should only be called after acquiring a
   // nsNSSShutDownPreventionLock. It's difficult to enforce this in code since
   // this is an implementation of an XPCOM interface function (albeit a
   // C++-only one), so we acquire the (reentrant) lock and check for shutdown
   // ourselves here. At the moment it appears that only nsCertTree uses this
   // function. When that gets removed and replaced by a more reasonable
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -232,18 +232,17 @@ collect_certs(void *arg, SECItem **certs
   return (SECSuccess);
 }
 
 CERTDERCerts*
 nsNSSCertificateDB::getCertsFromPackage(const UniquePLArenaPool& arena,
                                         uint8_t* data, uint32_t length,
                                         const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
-  CERTDERCerts* collectArgs =
-    (CERTDERCerts*)PORT_ArenaZAlloc(arena.get(), sizeof(CERTDERCerts));
+  CERTDERCerts* collectArgs = PORT_ArenaZNew(arena.get(), CERTDERCerts);
   if (!collectArgs) {
     return nullptr;
   }
 
   collectArgs->arena = arena.get();
   if (CERT_DecodeCertPackage(char_ptr_cast(data), length, collect_certs,
                              collectArgs) != SECSuccess) {
     return nullptr;
--- a/security/manager/ssl/nsNSSCertificateFakeTransport.cpp
+++ b/security/manager/ssl/nsNSSCertificateFakeTransport.cpp
@@ -420,17 +420,17 @@ nsNSSCertListFakeTransport::AddCert(nsIX
 
 NS_IMETHODIMP
 nsNSSCertListFakeTransport::DeleteCert(nsIX509Cert* aCert)
 {
   NS_NOTREACHED("Unimplemented on content process");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-void*
+CERTCertList*
 nsNSSCertListFakeTransport::GetRawCertList()
 {
   NS_NOTREACHED("Unimplemented on content process");
   return nullptr;
 }
 
 NS_IMETHODIMP
 nsNSSCertListFakeTransport::GetEnumerator(nsISimpleEnumerator**)
--- a/security/manager/ssl/nsNTLMAuthModule.cpp
+++ b/security/manager/ssl/nsNTLMAuthModule.cpp
@@ -488,17 +488,17 @@ ParseType2Msg(const void *inBuf, uint32_
   // read flags
   msg->flags = ReadUint32(cursor);
 
   // read challenge
   memcpy(msg->challenge, cursor, sizeof(msg->challenge));
   cursor += sizeof(msg->challenge);
 
   LOG(("NTLM type 2 message:\n"));
-  LogBuf("target", reinterpret_cast<const uint8_t*> (msg->target), msg->targetLen);
+  LogBuf("target", msg->target, msg->targetLen);
   LogBuf("flags", reinterpret_cast<const uint8_t*> (&msg->flags), 4);
   LogFlags(msg->flags);
   LogBuf("challenge", msg->challenge, sizeof(msg->challenge));
 
   // Read (and skip) the reserved field
   ReadUint32(cursor);
   ReadUint32(cursor);
   // Read target name security buffer: ...
@@ -783,17 +783,17 @@ GenerateType3Msg(const nsString &domain,
     rv = hasher->Update(msg.challenge, NTLM_CHAL_LEN);
     if (NS_FAILED(rv)) {
       return rv;
     }
     rv = hasher->Update(ntlmv2_blob1, NTLMv2_BLOB1_LEN);
     if (NS_FAILED(rv)) {
       return rv;
     }
-    rv = hasher->Update(reinterpret_cast<const uint8_t*> (msg.targetInfo), msg.targetInfoLen);
+    rv = hasher->Update(msg.targetInfo, msg.targetInfoLen);
     if (NS_FAILED(rv)) {
       return rv;
     }
     rv = hasher->Finish(false, ntlmv2ResponseStr);
     if (NS_FAILED(rv)) {
       return rv;
     }