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 idunknown
push userunknown
push dateunknown
reviewerscviecco
bugs1010634
milestone32.0a1
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