691997 - Code cleanup for Bug 172051 - Add localizable error messages for NSS error codes

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Wan-Teh Chang]

I'd like to suggest some cleanup changes for the patches in
bug 172051.  These cleanup changes should be made in NSS 3.13.1.

1. lib/nss/nssinit.c:

The #include "prerror.h" and #include "prtypes.h" that were
added in rev. 1.108 can be removed now:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nssinit.c&branch=&root=/cvsroot&subdir=mozilla/security/nss/lib/nss&command=DIFF_FRAMESET&rev1=1.107&rev2=1.108

2. lib/ssl/sslutil.h:

This header can be cvs removed.  The header declares two
functions:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslutil.h&rev=1.1

SSL_InitializePRErrorTable does not exist.
(ssl_InitializePRErrorTable exists but is declared in
sslerrstrs.)

ssl_Init should be declared in sslimpl.h.

3. lib/ssl/sslerrstrs.h

This header can be cvs removed.  The header declares only
one function:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/sslerrstrs.h&rev=1.1

ssl_InitializePRErrorTable should be declared in sslimpl.h.

Nit: ssl_InitializePRErrorTable should return SECStatus
instead of PRStatus.

Comment 1

[Elio Maldonado]

Attachment: Code cleanup requested

Comment 2

[Wan-Teh Chang]

Comment on attachment 568099 [details] [diff] [review]
Code cleanup requested

Review of attachment 568099 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.  Please make the following change before checking
this in.

::: mozilla/security/nss/lib/ssl/sslerrstrs.c
@@ -66,1 +66,2 @@
> >  }
> > +

Nit: remove this blank line.

::: mozilla/security/nss/lib/ssl/sslinit.c
@@ -55,1 +55,1 @@
> >  	   return (SEC_ERROR_NO_MEMORY);

This should be
    PORT_SetError(SEC_ERROR_NO_MEMORY);
    return SECFailure;

The setting of error code should ideally be done
by ssl_InitializePRErrorTable().

Note: The ssl_inited variable is not thread-safe and is
redundant with the use of PR_CallOnce in
ssl_InitializePRErrorTable.  It is ssl_Init that
should call PR_CallOnce.  You don't need to deal
with this in this patch.

Comment 3

[Elio Maldonado]

Changes checked in to trunk:
Checking in sslerrstrs.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslerrstrs.c,v  <--  sslerrstrs.c
new revision: 1.2; previous revision: 1.1
done
Removing sslerrstrs.h;
/cvsroot/mozilla/security/nss/lib/ssl/sslerrstrs.h,v  <--  sslerrstrs.h
new revision: delete; previous revision: 1.1
done
Checking in sslimpl.h;
/cvsroot/mozilla/security/nss/lib/ssl/sslimpl.h,v  <--  sslimpl.h
new revision: 1.84; previous revision: 1.83
done
Checking in sslinit.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslinit.c,v  <--  sslinit.c
new revision: 1.2; previous revision: 1.1
done
Checking in sslsnce.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslsnce.c,v  <--  sslsnce.c
new revision: 1.59; previous revision: 1.58
done
Checking in sslsock.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslsock.c,v  <--  sslsock.c
new revision: 1.75; previous revision: 1.74
done
Removing sslutil.h;
/cvsroot/mozilla/security/nss/lib/ssl/sslutil.h,v  <--  sslutil.h
new revision: delete; previous revision: 1.1
done

Comment 4

[Wan-Teh Chang]

Comment on attachment 568099 [details] [diff] [review]
Code cleanup requested

The CVS commit log in comment 3 is missing the lib/nss/nssinit.c
change in this patch.  I will open a new bug and check that in.