1 files changed, 28 insertions, 40 deletions

diff --git a/security/pkix/lib/pkixocsp.cpp b/security/pkix/lib/pkixocsp.cpp
index 215020f676..03f88bafcc 100644
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -1,31 +1,14 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* This code is made available to you under your choice of the following sets
- * of licensing terms:
- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/.
  */
-/* Copyright 2013 Mozilla Contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * 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 <limits>
 
 #include "pkix/pkix.h"
-#include "pkixcheck.h"
-#include "pkixutil.h"
+#include "pkix/pkixcheck.h"
+#include "pkix/pkixutil.h"
 
 namespace {
 
@@ -45,16 +28,16 @@ enum class CertStatus : uint8_t {
 class Context final
 {
 public:
-  Context(TrustDomain& trustDomain, const CertID& certID, Time time,
-          uint16_t maxLifetimeInDays, /*optional out*/ Time* thisUpdate,
-          /*optional out*/ Time* validThrough)
-    : trustDomain(trustDomain)
-    , certID(certID)
-    , time(time)
-    , maxLifetimeInDays(maxLifetimeInDays)
+  Context(TrustDomain& aTrustDomain, const CertID& aCertID, Time aTime,
+          uint16_t aMaxLifetimeInDays, /*optional out*/ Time* aThisUpdate,
+          /*optional out*/ Time* aValidThrough)
+    : trustDomain(aTrustDomain)
+    , certID(aCertID)
+    , time(aTime)
+    , maxLifetimeInDays(aMaxLifetimeInDays)
     , certStatus(CertStatus::Unknown)
-    , thisUpdate(thisUpdate)
-    , validThrough(validThrough)
+    , thisUpdate(aThisUpdate)
+    , validThrough(aValidThrough)
     , expired(false)
     , matchFound(false)
   {
@@ -172,9 +155,13 @@ static Result ExtensionNotUnderstood(Reader& extnID, Input extnValue,
 static Result RememberSingleExtension(Context& context, Reader& extnID,
                                       Input extnValue, bool critical,
                                       /*out*/ bool& understood);
-static inline Result CertID(Reader& input,
-                            const Context& context,
-                            /*out*/ bool& match);
+// It is convention to name the function after the part of the data structure
+// we're parsing from the RFC (e.g. OCSPResponse, ResponseBytes).
+// But since we also have a C++ type called CertID, this function doesn't
+// follow the convention to prevent shadowing.
+static inline Result MatchCertID(Reader& input,
+                                 const Context& context,
+                                 /*out*/ bool& match);
 static Result MatchKeyHash(TrustDomain& trustDomain,
                            Input issuerKeyHash,
                            Input issuerSubjectPublicKeyInfo,
@@ -437,12 +424,13 @@ BasicResponse(Reader& input, Context& context)
                      der::SEQUENCE, [&certs](Reader& certsDER) -> Result {
       while (!certsDER.AtEnd()) {
         Input cert;
-        Result rv = der::ExpectTagAndGetTLV(certsDER, der::SEQUENCE, cert);
-        if (rv != Success) {
-          return rv;
+        Result nestedRv =
+          der::ExpectTagAndGetTLV(certsDER, der::SEQUENCE, cert);
+        if (nestedRv != Success) {
+          return nestedRv;
         }
-        rv = certs.Append(cert);
-        if (rv != Success) {
+        nestedRv = certs.Append(cert);
+        if (nestedRv != Success) {
           return Result::ERROR_BAD_DER; // Too many certs
         }
       }
@@ -537,7 +525,7 @@ SingleResponse(Reader& input, Context& context)
 {
   bool match = false;
   Result rv = der::Nested(input, der::SEQUENCE, [&context, &match](Reader& r) {
-    return CertID(r, context, match);
+    return MatchCertID(r, context, match);
   });
   if (rv != Success) {
     return rv;
@@ -694,7 +682,7 @@ SingleResponse(Reader& input, Context& context)
 //        issuerKeyHash       OCTET STRING, -- Hash of issuer's public key
 //        serialNumber        CertificateSerialNumber }
 static inline Result
-CertID(Reader& input, const Context& context, /*out*/ bool& match)
+MatchCertID(Reader& input, const Context& context, /*out*/ bool& match)
 {
   match = false;
 
@@ -963,8 +951,8 @@ CreateEncodedOCSPRequest(TrustDomain& trustDomain, const struct CertID& certID,
   *d++ = 0x30; *d++ = totalLen - 10u; //         reqCert (CertID SEQUENCE)
 
   // reqCert.hashAlgorithm
-  for (size_t i = 0; i < sizeof(hashAlgorithm); ++i) {
-    *d++ = hashAlgorithm[i];
+  for (const uint8_t hashAlgorithmByte : hashAlgorithm) {
+    *d++ = hashAlgorithmByte;
   }
 
   // reqCert.issuerNameHash (OCTET STRING)