Bug 1010634, Part 1: Fix compiler warnings in certverifier, r=cviecco
authorBrian Smith <brian@briansmith.org>
Wed, 14 May 2014 17:46:32 -0700
changeset 183491 6deed68c2358d584d0988b200adc23fc88aa4113
parent 183490 a4ae7060f43ac1a4e49b30dfd7a95c5212940d4b
child 183492 ca3eb182bca034cc43d0f3318856145dc62f77f7
push id26793
push userryanvm@gmail.com
push dateFri, 16 May 2014 18:53:54 +0000
treeherderautoland@eb2a6f7785a2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs1010634
milestone32.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 1010634, Part 1: Fix compiler warnings in certverifier, r=cviecco
security/certverifier/ExtendedValidation.cpp
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/OCSPCache.cpp
security/certverifier/OCSPCache.h
--- a/security/certverifier/ExtendedValidation.cpp
+++ b/security/certverifier/ExtendedValidation.cpp
@@ -943,17 +943,18 @@ IdentityInfoInit()
       }
 #endif
       PR_NOT_REACHED("Could not find EV root in NSS storage");
       continue;
     }
 
     unsigned char certFingerprint[20];
     rv = PK11_HashBuf(SEC_OID_SHA1, certFingerprint,
-                      entry.cert->derCert.data, entry.cert->derCert.len);
+                      entry.cert->derCert.data,
+                      static_cast<int32_t>(entry.cert->derCert.len));
     PR_ASSERT(rv == SECSuccess);
     if (rv == SECSuccess) {
       bool same = !memcmp(certFingerprint, entry.ev_root_sha1_fingerprint, 20);
       PR_ASSERT(same);
       if (same) {
 
         SECItem ev_oid_item;
         ev_oid_item.data = nullptr;
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -463,28 +463,28 @@ NSSCertDBTrustDomain::IsChainValid(const
 }
 
 namespace {
 
 static char*
 nss_addEscape(const char* string, char quote)
 {
   char* newString = 0;
-  int escapes = 0, size = 0;
+  size_t escapes = 0, size = 0;
   const char* src;
   char* dest;
 
   for (src = string; *src; src++) {
   if ((*src == quote) || (*src == '\\')) {
     escapes++;
   }
   size++;
   }
 
-  newString = (char*) PORT_ZAlloc(escapes + size + 1);
+  newString = (char*) PORT_ZAlloc(escapes + size + 1u);
   if (!newString) {
     return nullptr;
   }
 
   for (src = string, dest = newString; *src; src++, dest++) {
     if ((*src == quote) || (*src == '\\')) {
       *dest++ = '\\';
     }
@@ -594,19 +594,19 @@ SetClassicOCSPBehavior(CertVerifier::ocs
 
   SEC_OcspFailureMode failureMode = strict == CertVerifier::ocsp_strict
                                   ? ocspMode_FailureIsVerificationFailure
                                   : ocspMode_FailureIsNotAVerificationFailure;
   (void) CERT_SetOCSPFailureMode(failureMode);
 
   CERT_ForcePostMethodForOCSP(get != CertVerifier::ocsp_get_enabled);
 
-  int OCSPTimeoutSeconds = 3;
+  uint32_t OCSPTimeoutSeconds = 3u;
   if (strict == CertVerifier::ocsp_strict) {
-    OCSPTimeoutSeconds = 10;
+    OCSPTimeoutSeconds = 10u;
   }
   CERT_SetOCSPTimeout(OCSPTimeoutSeconds);
 }
 
 char*
 DefaultServerNicknameForCert(CERTCertificate* cert)
 {
   char* nickname = nullptr;
--- a/security/certverifier/OCSPCache.cpp
+++ b/security/certverifier/OCSPCache.cpp
@@ -19,16 +19,18 @@
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include "OCSPCache.h"
 
+#include <limits>
+
 #include "NSSCertDBTrustDomain.h"
 #include "pk11pub.h"
 #include "secerr.h"
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gCertVerifierLog;
 #endif
 
@@ -108,40 +110,44 @@ OCSPCache::OCSPCache()
 {
 }
 
 OCSPCache::~OCSPCache()
 {
   Clear();
 }
 
-// Returns -1 if no entry is found for the given (cert, issuer) pair.
-int32_t
+// Returns false with index in an undefined state if no matching entry was
+// found.
+bool
 OCSPCache::FindInternal(const CERTCertificate* aCert,
                         const CERTCertificate* aIssuerCert,
+                        /*out*/ size_t& index,
                         const MutexAutoLock& /* aProofOfLock */)
 {
   if (mEntries.length() == 0) {
-    return -1;
+    return false;
   }
 
   SHA384Buffer idHash;
   SECStatus rv = CertIDHash(idHash, aCert, aIssuerCert);
   if (rv != SECSuccess) {
-    return -1;
+    return false;
   }
 
   // mEntries is sorted with the most-recently-used entry at the end.
   // Thus, searching from the end will often be fastest.
-  for (int32_t i = mEntries.length() - 1; i >= 0; i--) {
-    if (memcmp(mEntries[i]->mIDHash, idHash, SHA384_LENGTH) == 0) {
-      return i;
+  index = mEntries.length();
+  while (index > 0) {
+    --index;
+    if (memcmp(mEntries[index]->mIDHash, idHash, SHA384_LENGTH) == 0) {
+      return true;
     }
   }
-  return -1;
+  return false;
 }
 
 void
 OCSPCache::LogWithCerts(const char* aMessage, const CERTCertificate* aCert,
                         const CERTCertificate* aIssuerCert)
 {
 #ifdef PR_LOGGING
   if (PR_LOG_TEST(gCertVerifierLog, PR_LOG_DEBUG)) {
@@ -171,18 +177,18 @@ OCSPCache::Get(const CERTCertificate* aC
                PRErrorCode& aErrorCode,
                PRTime& aValidThrough)
 {
   PR_ASSERT(aCert);
   PR_ASSERT(aIssuerCert);
 
   MutexAutoLock lock(mMutex);
 
-  int32_t index = FindInternal(aCert, aIssuerCert, lock);
-  if (index < 0) {
+  size_t index;
+  if (!FindInternal(aCert, aIssuerCert, index, lock)) {
     LogWithCerts("OCSPCache::Get(%s, %s) not in cache", aCert, aIssuerCert);
     return false;
   }
   LogWithCerts("OCSPCache::Get(%s, %s) in cache", aCert, aIssuerCert);
   aErrorCode = mEntries[index]->mErrorCode;
   aValidThrough = mEntries[index]->mValidThrough;
   MakeMostRecentlyUsed(index, lock);
   return true;
@@ -195,19 +201,18 @@ OCSPCache::Put(const CERTCertificate* aC
                PRTime aThisUpdate,
                PRTime aValidThrough)
 {
   PR_ASSERT(aCert);
   PR_ASSERT(aIssuerCert);
 
   MutexAutoLock lock(mMutex);
 
-  int32_t index = FindInternal(aCert, aIssuerCert, lock);
-
-  if (index >= 0) {
+  size_t index;
+  if (FindInternal(aCert, aIssuerCert, index, lock)) {
     // Never replace an entry indicating a revoked certificate.
     if (mEntries[index]->mErrorCode == SEC_ERROR_REVOKED_CERTIFICATE) {
       LogWithCerts("OCSPCache::Put(%s, %s) already in cache as revoked - "
                    "not replacing", aCert, aIssuerCert);
       MakeMostRecentlyUsed(index, lock);
       return SECSuccess;
     }
 
--- a/security/certverifier/OCSPCache.h
+++ b/security/certverifier/OCSPCache.h
@@ -87,19 +87,20 @@ private:
     PRTime mThisUpdate;
     PRTime mValidThrough;
     // The SHA-384 hash of the concatenation of the DER encodings of the
     // issuer name and issuer key, followed by the serial number.
     // See the documentation for CertIDHash in OCSPCache.cpp.
     SHA384Buffer mIDHash;
   };
 
-  int32_t FindInternal(const CERTCertificate* aCert,
-                       const CERTCertificate* aIssuerCert,
-                       const MutexAutoLock& aProofOfLock);
+  bool FindInternal(const CERTCertificate* aCert,
+                    const CERTCertificate* aIssuerCert,
+                    /*out*/ size_t& index,
+                    const MutexAutoLock& aProofOfLock);
   void MakeMostRecentlyUsed(size_t aIndex, const MutexAutoLock& aProofOfLock);
   void LogWithCerts(const char* aMessage, const CERTCertificate* aCert,
                     const CERTCertificate* aIssuerCert);
 
   Mutex mMutex;
   static const size_t MaxEntries = 1024;
   // Sorted with the most-recently-used entry at the end.
   // Using 256 here reserves as much possible inline storage as the vector