Bug 1271501 - Remove unnecessary uses of reinterpret_cast in PSM. r=keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Wed, 18 May 2016 18:58:40 -0700
changeset 368590 e6ef5c1febbabcc0366e10703be7cd5a00ec918a
parent 368550 113aed339ad20300dcd8d420b04a759c01f84158
child 368591 857796af22807857af260f0cc00bf7304b76dbd5
push id18597
push usercykesiopka.bmo@gmail.com
push dateThu, 19 May 2016 04:17:43 +0000
reviewerskeeler
bugs1271501
milestone49.0a1
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;
     }