337080 - Coverity 681, dead code in mozilla/security/nss/lib/ssl/ssl3con.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Jon Smirl]

Clean up error path to remove dead code. So far I have found one dead code flag that was a bug. If these are cleaned Coverity won't keep hitting them.

diff -u -r1.76 ssl3con.c
--- security/nss/lib/ssl/ssl3con.c      18 Nov 2005 01:25:20 -0000      1.76
+++ security/nss/lib/ssl/ssl3con.c      8 May 2006 03:52:30 -0000
@@ -1849,16 +1849,12 @@
            PORT_Assert(rv == SECSuccess && cipherBytesPart2 == p2Len);
            if (rv != SECSuccess || cipherBytesPart2 != p2Len) {
                PORT_SetError(SSL_ERROR_ENCRYPTION_FAILURE);
-                   goto spec_locked_loser;
-               }
+spec_locked_loser:
+               ssl_ReleaseSpecReadLock(ss);
+               return SECFailure;
+           }
            cipherBytes += cipherBytesPart2;
        }
-       if (rv != SECSuccess) {
-           ssl_MapLowLevelError(SSL_ERROR_ENCRYPTION_FAILURE);
-spec_locked_loser:
-           ssl_ReleaseSpecReadLock(ss);
-           return SECFailure;
-       }
        PORT_Assert(cipherBytes <= MAX_FRAGMENT_LENGTH + 1024);

        /*

Comment 1

[Jon Smirl]

Attachment: Clean up error path

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 221256 [details] [diff] [review]
Clean up error path

There's no incorrect behavior here, nothing that produces wrong results.  
Moving the common code into one of the cases that invokes it doesn't make the code clearer or more maintainable.

Comment 3

[Jon Smirl]

These two lines are unreachable.

-       if (rv != SECSuccess) {
-           ssl_MapLowLevelError(SSL_ERROR_ENCRYPTION_FAILURE);

When you remove them it makes sense to move the label and the code.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

(In reply to comment #3)
> These two lines are unreachable.
> 
> -       if (rv != SECSuccess) {
> -           ssl_MapLowLevelError(SSL_ERROR_ENCRYPTION_FAILURE);

try replacing them with 
> +       if (0) {

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: YA alternative

Alexei, please review

Comment 6

[Alexei Volkov]

Comment on attachment 222467 [details] [diff] [review]
YA alternative

r=alexei.volkov.bugs

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Checking in ssl/ssl3con.c; new revision: 1.90; previous revision: 1.89
Checking in ssl3con.c; new revision: 1.76.2.12; previous revision: 1.76.2.11

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

CID 160 & 681

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Also CID 853

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Also CID 952