Bug 1589073 - Use of new PR_ASSERT_ARG in certdb.c. r=mt
authorMarcus Burghardt <mburghardt@mozilla.com>
Tue, 05 Nov 2019 09:44:51 +0000
changeset 15374 73c28cad3dbb7c8eef8f970d69fc5504d96b69dc
parent 15373 0a86945adf746d78954c4a5ac4dfa365debb76c6
child 15375 3d7e509d6d20ecd607a28fa6ce42e4ffd9c51443
push id3567
push usermthomson@mozilla.com
push dateTue, 05 Nov 2019 22:59:46 +0000
reviewersmt
bugs1589073, 1588015
Bug 1589073 - Use of new PR_ASSERT_ARG in certdb.c. r=mt Bug 1588015 introduced in NSPR a new way to ASSERT values where the arguments are always used avoiding "unused variable" errors. This was implemented in NSS, at certdb.c. Differential Revision: https://phabricator.services.mozilla.com/D49418
lib/certdb/certdb.c
lib/util/secport.h
--- a/lib/certdb/certdb.c
+++ b/lib/certdb/certdb.c
@@ -2885,19 +2885,17 @@ CERT_LockCertRefCount(CERTCertificate *c
 /*
  * Free the cert reference count lock
  */
 void
 CERT_UnlockCertRefCount(CERTCertificate *cert)
 {
     PORT_Assert(certRefCountLock != NULL);
     PRStatus prstat = PZ_Unlock(certRefCountLock);
-    if (prstat != PR_SUCCESS) {
-        PORT_Assert(prstat == PR_SUCCESS);
-    }
+    PORT_AssertArg(prstat == PR_SUCCESS);
 }
 
 static PZLock *certTrustLock = NULL;
 
 /*
  * Acquire the cert trust lock
  * There is currently one global lock for all certs, but I'm putting a cert
  * arg here so that it will be easy to make it per-cert in the future if
@@ -2992,32 +2990,28 @@ cert_DestroyLocks(void)
 /*
  * Free the cert trust lock
  */
 void
 CERT_UnlockCertTrust(const CERTCertificate *cert)
 {
     PORT_Assert(certTrustLock != NULL);
     PRStatus prstat = PZ_Unlock(certTrustLock);
-    if (prstat != PR_SUCCESS) {
-        PORT_Assert(prstat == PR_SUCCESS);
-    }
+    PORT_AssertArg(prstat == PR_SUCCESS);
 }
 
 /*
  * Free the temp/perm lock
  */
 void
 CERT_UnlockCertTempPerm(const CERTCertificate *cert)
 {
     PORT_Assert(certTempPermLock != NULL);
     PRStatus prstat = PZ_Unlock(certTempPermLock);
-    if (prstat != PR_SUCCESS) {
-        PORT_Assert(prstat == PR_SUCCESS);
-    }
+    PORT_AssertArg(prstat == PR_SUCCESS);
 }
 
 /*
  * Get the StatusConfig data for this handle
  */
 CERTStatusConfig *
 CERT_GetStatusConfig(CERTCertDBHandle *handle)
 {
--- a/lib/util/secport.h
+++ b/lib/util/secport.h
@@ -116,16 +116,21 @@ extern void *PORT_ArenaMark(PLArenaPool 
 extern void PORT_ArenaRelease(PLArenaPool *arena, void *mark);
 extern void PORT_ArenaZRelease(PLArenaPool *arena, void *mark);
 extern void PORT_ArenaUnmark(PLArenaPool *arena, void *mark);
 extern char *PORT_ArenaStrdup(PLArenaPool *arena, const char *str);
 
 SEC_END_PROTOS
 
 #define PORT_Assert PR_ASSERT
+/* This is a variation of PORT_Assert where the arguments will be always
+ * used either in Debug or not. But, in optimized mode the result will be
+ * ignored. See more details in Bug 1588015. */
+#define PORT_AssertArg PR_ASSERT_ARG
+
 /* This runs a function that should return SECSuccess.
  * Intended for NSS internal use only.
  * The return value is asserted in a debug build, otherwise it is ignored.
  * This is no substitute for proper error handling.  It is OK only if you
  * have ensured that the function cannot fail by other means such as checking
  * prerequisites.  In that case this can be used as a safeguard against
  * unexpected changes in a function.
  */