Bug 1252722 - Ensure arguments of all public methods are checked. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Thu, 31 Mar 2016 17:32:53 -0700
changeset 291233 0f8ee183f2cde3efd970c06efc45d6e69502cbb0
parent 291232 64c333e9b12a998019e80654b75686a1ce644aeb
child 291234 71b02a7c2340ce884f3650976621eb7c9ab10051
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1252722
milestone48.0a1
Bug 1252722 - Ensure arguments of all public methods are checked. r=keeler MozReview-Commit-ID: 5UJup8k8iGe
security/manager/ssl/nsPKCS11Slot.cpp
security/manager/ssl/nsPKCS11Slot.h
--- a/security/manager/ssl/nsPKCS11Slot.cpp
+++ b/security/manager/ssl/nsPKCS11Slot.cpp
@@ -12,18 +12,20 @@
 #include "secmod.h"
 
 using mozilla::LogLevel;
 
 extern mozilla::LazyLogModule gPIPNSSLog;
 
 NS_IMPL_ISUPPORTS(nsPKCS11Slot, nsIPKCS11Slot)
 
-nsPKCS11Slot::nsPKCS11Slot(PK11SlotInfo *slot)
+nsPKCS11Slot::nsPKCS11Slot(PK11SlotInfo* slot)
 {
+  MOZ_ASSERT(slot);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
 
   mSlot.reset(PK11_ReferenceSlot(slot));
   mSeries = PK11_GetSlotSeries(slot);
   refreshSlotInfo(locker);
 }
@@ -70,29 +72,33 @@ nsPKCS11Slot::~nsPKCS11Slot()
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return;
   }
   destructorSafeDestroyNSSReference();
   shutdown(calledFromObject);
 }
 
-void nsPKCS11Slot::virtualDestroyNSSReference()
+void
+nsPKCS11Slot::virtualDestroyNSSReference()
 {
   destructorSafeDestroyNSSReference();
 }
 
-void nsPKCS11Slot::destructorSafeDestroyNSSReference()
+void
+nsPKCS11Slot::destructorSafeDestroyNSSReference()
 {
   mSlot = nullptr;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetName(char16_t **aName)
+NS_IMETHODIMP
+nsPKCS11Slot::GetName(char16_t** aName)
 {
+  NS_ENSURE_ARG_POINTER(aName);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   // |csn| is non-owning.
   char* csn = PK11_GetSlotName(mSlot.get());
   if (*csn) {
     *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(csn));
@@ -104,95 +110,107 @@ nsPKCS11Slot::GetName(char16_t **aName)
   } else {
     // same as above, this is a catch-all
     *aName = ToNewUnicode(NS_LITERAL_STRING("Unnamed Slot"));
   }
   if (!*aName) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetDesc(char16_t **aDesc)
+NS_IMETHODIMP
+nsPKCS11Slot::GetDesc(char16_t** aDesc)
 {
+  NS_ENSURE_ARG_POINTER(aDesc);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
     refreshSlotInfo(locker);
   }
 
   *aDesc = ToNewUnicode(mSlotDesc);
   if (!*aDesc) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetManID(char16_t **aManID)
+NS_IMETHODIMP
+nsPKCS11Slot::GetManID(char16_t** aManID)
 {
+  NS_ENSURE_ARG_POINTER(aManID);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
     refreshSlotInfo(locker);
   }
   *aManID = ToNewUnicode(mSlotManID);
   if (!*aManID) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetHWVersion(char16_t **aHWVersion)
+NS_IMETHODIMP
+nsPKCS11Slot::GetHWVersion(char16_t** aHWVersion)
 {
+  NS_ENSURE_ARG_POINTER(aHWVersion);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
     refreshSlotInfo(locker);
   }
   *aHWVersion = ToNewUnicode(mSlotHWVersion);
   if (!*aHWVersion) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetFWVersion(char16_t **aFWVersion)
+NS_IMETHODIMP
+nsPKCS11Slot::GetFWVersion(char16_t** aFWVersion)
 {
+  NS_ENSURE_ARG_POINTER(aFWVersion);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   if (PK11_GetSlotSeries(mSlot.get()) != mSeries) {
     refreshSlotInfo(locker);
   }
   *aFWVersion = ToNewUnicode(mSlotFWVersion);
   if (!*aFWVersion) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetToken(nsIPK11Token **_retval)
+NS_IMETHODIMP
+nsPKCS11Slot::GetToken(nsIPK11Token** _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   nsCOMPtr<nsIPK11Token> token = new nsPK11Token(mSlot.get());
   token.forget(_retval);
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Slot::GetTokenName(char16_t **aName)
+NS_IMETHODIMP
+nsPKCS11Slot::GetTokenName(char16_t** aName)
 {
+  NS_ENSURE_ARG_POINTER(aName);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if (!PK11_IsPresent(mSlot.get())) {
     *aName = nullptr;
     return NS_OK;
   }
@@ -202,18 +220,20 @@ nsPKCS11Slot::GetTokenName(char16_t **aN
   }
 
   *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(PK11_GetTokenName(mSlot.get())));
   if (!*aName) return NS_ERROR_OUT_OF_MEMORY;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsPKCS11Slot::GetStatus(uint32_t *_retval)
+nsPKCS11Slot::GetStatus(uint32_t* _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if (PK11_IsDisabled(mSlot.get())) {
     *_retval = SLOT_DISABLED;
   } else if (!PK11_IsPresent(mSlot.get())) {
     *_retval = SLOT_NOT_PRESENT;
@@ -227,18 +247,20 @@ nsPKCS11Slot::GetStatus(uint32_t *_retva
   } else {
     *_retval = SLOT_READY;
   }
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS(nsPKCS11Module, nsIPKCS11Module)
 
-nsPKCS11Module::nsPKCS11Module(SECMODModule *module)
+nsPKCS11Module::nsPKCS11Module(SECMODModule* module)
 {
+  MOZ_ASSERT(module);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
 
   mModule.reset(SECMOD_ReferenceModule(module));
 }
 
 nsPKCS11Module::~nsPKCS11Module()
@@ -246,56 +268,64 @@ nsPKCS11Module::~nsPKCS11Module()
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return;
   }
   destructorSafeDestroyNSSReference();
   shutdown(calledFromObject);
 }
 
-void nsPKCS11Module::virtualDestroyNSSReference()
+void
+nsPKCS11Module::virtualDestroyNSSReference()
 {
   destructorSafeDestroyNSSReference();
 }
 
-void nsPKCS11Module::destructorSafeDestroyNSSReference()
+void
+nsPKCS11Module::destructorSafeDestroyNSSReference()
 {
   mModule = nullptr;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Module::GetName(char16_t **aName)
+NS_IMETHODIMP
+nsPKCS11Module::GetName(char16_t** aName)
 {
+  NS_ENSURE_ARG_POINTER(aName);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(mModule->commonName));
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Module::GetLibName(char16_t **aName)
+NS_IMETHODIMP
+nsPKCS11Module::GetLibName(char16_t** aName)
 {
+  NS_ENSURE_ARG_POINTER(aName);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   if ( mModule->dllName ) {
     *aName = ToNewUnicode(NS_ConvertUTF8toUTF16(mModule->dllName));
   } else {
     *aName = nullptr;
   }
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11Module::FindSlotByName(const char16_t *aName,
-                               nsIPKCS11Slot **_retval)
+NS_IMETHODIMP
+nsPKCS11Module::FindSlotByName(const char16_t* aName, nsIPKCS11Slot** _retval)
 {
+  // Note: It's OK for |aName| to be null.
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
 
   NS_ConvertUTF16toUTF8 asciiname(aName);
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Getting \"%s\"\n", asciiname.get()));
   UniquePK11SlotInfo slotInfo;
   UniquePK11SlotList slotList(PK11_FindSlotsByNames(mModule->dllName,
@@ -323,16 +353,18 @@ nsPKCS11Module::FindSlotByName(const cha
   nsCOMPtr<nsIPKCS11Slot> slot = new nsPKCS11Slot(slotInfo.get());
   slot.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPKCS11Module::ListSlots(nsISimpleEnumerator** _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID);
   if (!array) {
     return NS_ERROR_FAILURE;
@@ -367,58 +399,65 @@ nsPKCS11ModuleDB::~nsPKCS11ModuleDB()
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return;
   }
 
   shutdown(calledFromObject);
 }
 
-NS_IMETHODIMP 
-nsPKCS11ModuleDB::GetInternal(nsIPKCS11Module **_retval)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::GetInternal(nsIPKCS11Module** _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   UniqueSECMODModule nssMod(
     SECMOD_CreateModule(nullptr, SECMOD_INT_NAME, nullptr, SECMOD_INT_FLAGS));
   if (!nssMod) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(nssMod.get());
   module.forget(_retval);
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11ModuleDB::GetInternalFIPS(nsIPKCS11Module **_retval)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::GetInternalFIPS(nsIPKCS11Module** _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   UniqueSECMODModule nssMod(
     SECMOD_CreateModule(nullptr, SECMOD_FIPS_NAME, nullptr, SECMOD_FIPS_FLAGS));
   if (!nssMod) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(nssMod.get());
   module.forget(_retval);
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsPKCS11ModuleDB::FindModuleByName(const char16_t *aName,
-                                   nsIPKCS11Module **_retval)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::FindModuleByName(const char16_t* aName,
+                                   nsIPKCS11Module** _retval)
 {
+  // Note: It's OK for |aName| to be null.
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ConvertUTF16toUTF8 utf8Name(aName);
   UniqueSECMODModule mod(SECMOD_FindModule(const_cast<char*>(utf8Name.get())));
   if (!mod) {
@@ -428,20 +467,23 @@ nsPKCS11ModuleDB::FindModuleByName(const
   nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(mod.get());
   module.forget(_retval);
   return NS_OK;
 }
 
 /* This is essentially the same as nsIPK11Token::findTokenByName, except
  * that it returns an nsIPKCS11Slot, which may be desired.
  */
-NS_IMETHODIMP 
-nsPKCS11ModuleDB::FindSlotByName(const char16_t *aName,
-                                 nsIPKCS11Slot **_retval)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::FindSlotByName(const char16_t* aName,
+                                 nsIPKCS11Slot** _retval)
 {
+  // Note: It's OK for |aName| to be null.
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ConvertUTF16toUTF8 utf8Name(aName);
   UniquePK11SlotInfo slotInfo(
     PK11_FindSlotByName(const_cast<char*>(utf8Name.get())));
@@ -452,16 +494,18 @@ nsPKCS11ModuleDB::FindSlotByName(const c
   nsCOMPtr<nsIPKCS11Slot> slot = new nsPKCS11Slot(slotInfo.get());
   slot.forget(_retval);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPKCS11ModuleDB::ListModules(nsISimpleEnumerator** _retval)
 {
+  NS_ENSURE_ARG_POINTER(_retval);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID);
   if (!array) {
     return NS_ERROR_FAILURE;
@@ -486,66 +530,72 @@ nsPKCS11ModuleDB::ListModules(nsISimpleE
     if (NS_FAILED(rv)) {
       return rv;
     }
   }
 
   return array->Enumerate(_retval);
 }
 
-NS_IMETHODIMP nsPKCS11ModuleDB::GetCanToggleFIPS(bool *aCanToggleFIPS)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::GetCanToggleFIPS(bool* aCanToggleFIPS)
 {
+  NS_ENSURE_ARG_POINTER(aCanToggleFIPS);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   *aCanToggleFIPS = SECMOD_CanDeleteInternalModule();
   return NS_OK;
 }
 
 
-NS_IMETHODIMP nsPKCS11ModuleDB::ToggleFIPSMode()
+NS_IMETHODIMP
+nsPKCS11ModuleDB::ToggleFIPSMode()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  // The way to toggle FIPS mode in NSS is extremely obscure.
-  // Basically, we delete the internal module, and voila it
-  // gets replaced with the opposite module, ie if it was 
-  // FIPS before, then it becomes non-FIPS next.
-  SECMODModule *internal;
+  // The way to toggle FIPS mode in NSS is extremely obscure. Basically, we
+  // delete the internal module, and it gets replaced with the opposite module
+  // (i.e. if it was FIPS before, then it becomes non-FIPS next).
+  // SECMOD_GetInternalModule() returns a pointer to a local copy of the
+  // internal module stashed in NSS.  We don't want to delete it since it will
+  // cause much pain in NSS.
+  SECMODModule* internal = SECMOD_GetInternalModule();
+  if (!internal) {
+    return NS_ERROR_FAILURE;
+  }
 
-  // This function returns us a pointer to a local copy of 
-  // the internal module stashed in NSS.  We don't want to
-  // delete it since it will cause much pain in NSS.
-  internal = SECMOD_GetInternalModule();
-  if (!internal)
+  if (SECMOD_DeleteInternalModule(internal->commonName) != SECSuccess) {
     return NS_ERROR_FAILURE;
-
-  SECStatus srv = SECMOD_DeleteInternalModule(internal->commonName);
-  if (srv != SECSuccess)
-    return NS_ERROR_FAILURE;
+  }
 
   if (PK11_IsFIPS()) {
     Telemetry::Accumulate(Telemetry::FIPS_ENABLED, true);
   }
 
   return NS_OK;
 }
 
-NS_IMETHODIMP nsPKCS11ModuleDB::GetIsFIPSEnabled(bool *aIsFIPSEnabled)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::GetIsFIPSEnabled(bool* aIsFIPSEnabled)
 {
+  NS_ENSURE_ARG_POINTER(aIsFIPSEnabled);
+
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   *aIsFIPSEnabled = PK11_IsFIPS();
   return NS_OK;
 }
 
-NS_IMETHODIMP nsPKCS11ModuleDB::GetIsFIPSModeActive(bool *aIsFIPSModeActive)
+NS_IMETHODIMP
+nsPKCS11ModuleDB::GetIsFIPSModeActive(bool* aIsFIPSModeActive)
 {
   return GetIsFIPSEnabled(aIsFIPSModeActive);
 }
--- a/security/manager/ssl/nsPKCS11Slot.h
+++ b/security/manager/ssl/nsPKCS11Slot.h
@@ -19,17 +19,17 @@
 
 class nsPKCS11Slot : public nsIPKCS11Slot,
                      public nsNSSShutDownObject
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPKCS11SLOT
 
-  explicit nsPKCS11Slot(PK11SlotInfo *slot);
+  explicit nsPKCS11Slot(PK11SlotInfo* slot);
 
 protected:
   virtual ~nsPKCS11Slot();
 
 private:
   mozilla::UniquePK11SlotInfo mSlot;
   nsString mSlotDesc, mSlotManID, mSlotHWVersion, mSlotFWVersion;
   int mSeries;
@@ -41,17 +41,17 @@ private:
 
 class nsPKCS11Module : public nsIPKCS11Module,
                        public nsNSSShutDownObject
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPKCS11MODULE
 
-  explicit nsPKCS11Module(SECMODModule *module);
+  explicit nsPKCS11Module(SECMODModule* module);
 
 protected:
   virtual ~nsPKCS11Module();
 
 private:
   mozilla::UniqueSECMODModule mModule;
 
   virtual void virtualDestroyNSSReference() override;