336938 - Coverity 170, dead code in mozilla/security/nss/lib/freebl/pqg.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Jon Smirl]

PQG_VerifyParams()
{
    if (!params || !vfy || !result) {
	PORT_SetError(SEC_ERROR_INVALID_ARGS);
	return SECFailure;
    }
// vfy is checked to be not null at top of routine
// Later check is redundant since variable is not changed

@@ -628,7 +628,6 @@
     /* 6.  P is prime */
     CHECKPARAM( mpp_pprime(&P, PQG_P_PRIMALITY_TESTS) == MP_YES );
     /* Steps 7-12 are done only if the optional PQGVerify is supplied. */
-    if (!vfy) goto cleanup;
     /* 7.  counter < 4096 */
     CHECKPARAM( vfy->counter < 4096 );
     /* 8.  g >= 160 and g < 2048   (g is length of seed in bits) */

Comment 1

[Jon Smirl]

Attachment: Remove dead code

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 221128 [details] [diff] [review]
Remove dead code

This is indeed dead code.
But the only reason to remove it is to silence coverity.  It does no harm.
It causes no incorrect behavior or results.
And this change is in code that gets reviewed by NIST.  
We don't want to make changes that trigger unneccessary NIST code reviews, and IMO, this proposed change is exactly such an unnecessary change.
I'll leave this bug for Wan-Teh to decide (he's the guy working on getting the code through NIST code reviews).  
But IMO, this patch may as well be r-.

Comment 3

[Wan-Teh Chang]

I checked in the patch on the NSS trunk (NSS 3.12) and
NSS_3_11_BRANCH (3.11.2).

We haven't passed NIST's PQG Generation test yet (bug
334533), so it is fine to make changes to this file now.
However, we plan to send the NSS source code to the testing
lab for review this afternoon or on Monday,  so today is
the last day we will make non-critical bug fixes in
mozilla/security/nss/lib/{freebl,softoken} on the
NSS_3_11_BRANCH (which is what will go into Firefox 2.0).