290238 - Do not export pk11pub.h and pk11priv.h.

Status: RESOLVED WONTFIX

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

I raised this issue by email before.

In NSS 3.10, we divided pk11func.h into two halves:
pk11pub.h and pk11priv.h.  pk11pub.h declares the
public functions.  pk11priv.h declares the private
functions.  pk11func.h was changed to simply include
pk11pub.h and pk11priv.h.  Since pk11func.h is exported,
this requires both pk11pub.h and pk11priv.h be exported.

We should not export a header that declares private
functions.  If our clients cannot compile their code
without including pk11priv.h, I won't understand why.

If you agree pk11priv.h should not be exported, then
pk11pub.h becomes redundant and can be deleted.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

With this patch applied, NSS compiles with no undeclared
function compiler warnings on Linux and Windows.

1. cvs remove pk11pub.h.
2. pk11func.h now contains the contents of pk11pub.h.
3. make pk11priv.h a private export because it is included
by a few files outside lib/pk11wrap.  Note that in the cmd
directory, only a single file, cmd/signtool/sign.c, needs
to include pk11priv.h.	This supports my position that our
customers don't need to include pk11priv.h.
4. add #include "pk11priv.h" where necessary.
5. modify Solaris pkg file to exclude pk11pub.h and pk11priv.h
from the SUNWtlsd package.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

I'm not sure, but I think you're proposing to remove one or more files that
are now public, yes?  It is now politically infeasible within Sun to remove 
any public files (even ones that are provably unused) from 3.10.  It would be
ruinous to us to make ANY changes that have ANY chance of breaking the build 
of some NSS 3.10 dependent product or that necessitate any code changes in
those products.  That is why the fortezza removal patch is not going into 
NSS 3.10.  So, please don't remove any public headers from 3.10 (even if they
arguably cannot be used).

I think it will be fine to do this (and the fortezza removal) for NSS 3.11.  
We should talk about 3.11 plans.

Comment 3

[Wan-Teh Chang]

pk11pub.h and pk11priv.h were added during NSS 3.10
development.

Based on my experience with NSS Beta customers, I
guarantee that no NSS 3.10 Beta customers have modified
their code to include pk11pub.h or pk11priv.h because
their code can compile without any modifications.  So
removing these two exported headers now has no risk.

Comment 4

[Robert Relyea]

I'm not sure I agree with the logic in pk11pub.h.
pk11pub.h should be exported.
pk11func.h is depricated, but needs to be included for old programs.

The question is what to do about pk11priv.h. Even though those functions are
private (cannot be called outside of the nss3.dll), the were once part of
pk11func.h (NSS 3.9). It is possible that programs included references to these
functions, but never linked them (or linked them to some private
implementation). it's convoluted, but possible. That is why I included
pk11priv.h as a public header and added it as an include to pk11func.h.

Within NSS pk11func.h should be phased out in favor of pk11pub.h

In an ideal world pk11func.h and pk11priv.h would disappear. I'm not sure we are
in that would. At any rate pk11pub.h should always be exported, public and the
one that we suggest clients use.

bob


 Unless you change pk11func.h to contain all of pk11pub.h, then

Comment 5

[Robert Relyea]

Comment on attachment 180637 [details] [diff] [review]
Proposed patch

r- pk11func.h should go away, not pk11pub.h

Comment 6

[Wan-Teh Chang]

Bob,

The fundamental question is whether we should export
pk11priv.h.  If we export pk11priv.h, then I agree that
we must deprecate pk11func.h and suggest our clients use
pk11pub.h.  But I think pk11priv.h should not be exported.

We have done the equivalent of not exporting pk11priv.h
in the past -- we routinely move declarations of private
functions from public headers to private headers (often
converting their prefixes to lowercase at the same time).
This is why I think we don't need to export pk11priv.h.

If we don't export pk11priv.h, pk11func.h and pk11pub.h
will be the same and we only need to keep one of them.
The reason I choose pk11func.h over pk11pub.h is that our
documentation refers to pk11func.h (for example,
http://www.mozilla.org/projects/security/pki/nss/ref/ssl/pkfnc.html),
and there is nothing wrong with the pk11func.h file name.
If we deprecate pk11func.h, we will need to update our
documentation.

So, the question is do we really need to export pk11priv.h?

Comment 7

[Wan-Teh Chang]

Marked the bug WONTFIX.  I can live with this.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 180637 [details] [diff] [review]
Proposed patch

removing sr request since the bug is now WONTFIX.