Last Comment Bug 118830 - NSS public header files should be C++ safe
: NSS public header files should be C++ safe
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.3.1
: All All
: P3 minor (vote)
: 3.12
Assigned To: Julien Pierre
:
Mentors:
Depends on: 388120
Blocks:
  Show dependency treegraph
 
Reported: 2002-01-08 13:26 PST by Wan-Teh Chang
Modified: 2007-07-16 23:30 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add SEC_BEGIN_PROTOS / SEC_END_PROTOS (1.11 KB, patch)
2007-07-10 19:02 PDT, Julien Pierre
nelson: review+
Details | Diff | Review

Description Wan-Teh Chang 2002-01-08 13:26:04 PST
NSS public header files should be able to be safely included
by a C++ file without the need for an extern "C" { } block.

I just noticed that several NSS headers are included by PSM
files (C++) in extern "C" blocks:

pkcs11.h
pkcs12.h
p12plcy.h
pk11pqg.h
secdert.h
keydbt.h
crmf.h

We should get them fixed.
Comment 1 Julien Pierre 2002-01-08 14:15:46 PST
Wan-Teh,

Did you try to remove the extern "C" and build PSM ?
I know there have been issues in the past with headers' C++ safety but many got
resolved. Usually a temporary workaround was to put extern "C" before fixes were
made. That was certainly the case in the web server, and those workarounds were
never removed since their presence did not harm anything in the build process. 
Comment 2 Wan-Teh Chang 2002-01-08 14:21:02 PST
No, I didn't try to remove the extern "C" and see if it works.

I filed this bug so that I don't forget about this problem.
It is low priority and has an easy workaround.
Comment 3 Wan-Teh Chang 2002-04-25 16:11:47 PDT
Changed the QA contact to Bishakha.
Comment 4 Wan-Teh Chang 2002-05-08 17:04:15 PDT
Set target milestone to NSS 3.5.
Comment 5 Wan-Teh Chang 2002-12-06 11:09:47 PST
Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.
Comment 6 Nelson Bolyard (seldom reads bugmail) 2003-05-09 21:13:59 PDT
Remove target milestone of 3.8, since these bugs didn't get into that release.
Comment 7 Julien Pierre 2007-07-10 19:01:39 PDT
I checked all the header files named above. keydbt.h no longer exists. All the others have been fixed and are C++-safe, except for secdert.h which has some data symbols outside that are not in a SEC_BEGIN_PROTOS / SEC_END_PROTOS block . Somehow I doubt this really poses much of a problem because the data symbols wouldn't even be visible on Windows, so the name mangling would probably not even come into play. But just for consistency, I will provide a patch for that header.
Comment 8 Julien Pierre 2007-07-10 19:02:13 PDT
Created attachment 271781 [details] [diff] [review]
Add SEC_BEGIN_PROTOS / SEC_END_PROTOS
Comment 9 Nelson Bolyard (seldom reads bugmail) 2007-07-10 20:52:46 PDT
Comment on attachment 271781 [details] [diff] [review]
Add SEC_BEGIN_PROTOS / SEC_END_PROTOS

Doesn't the ``extern "C" { '' need to enclose all the typedefs and struct
type declarations, to prevent their type names from being mangled, and
to prevent the structs from being treated as public classes?  
If so, then the BEGIN_PROTOS is in the wrong place.
If not, then this is OK, but I'd prefer that the BEGIN_PROTOS be put at 
the front of the file anyway.
Comment 10 Julien Pierre 2007-07-10 22:00:30 PDT
Nelson,

extern "C" doesn't have any effect on typedefs and structs, it is only for function and variable symbol names, but it is harmless to move it up.
Comment 11 Nelson Bolyard (seldom reads bugmail) 2007-07-11 10:41:32 PDT
Comment on attachment 271781 [details] [diff] [review]
Add SEC_BEGIN_PROTOS / SEC_END_PROTOS

r=nelson
Move BEGIN_PROTOS up or not as you see fit.
Comment 12 Wan-Teh Chang 2007-07-11 11:41:42 PDT
Right.  I observed that NSS headers have been careful to
exclude typedefs, structs, and macros from extern "C".
So it is a good idea to preserve that convention.
Comment 13 Julien Pierre 2007-07-11 16:38:30 PDT
Thanks for the reviews. I checked in the patch as-is to the trunk.

Checking in secdert.h;
/cvsroot/mozilla/security/nss/lib/util/secdert.h,v  <--  secdert.h
new revision: 1.3; previous revision: 1.2
done
Comment 14 Kai Engert (:kaie) 2007-07-13 13:08:34 PDT
This patch breaks compilation of PSM-with-NSS-3.12

nsKeygenHandler.cpp
../../../../dist/public/nss/secdert.h:163: error: expected constructor, destructor, or type conversion before ‘extern’
../../../../dist/include/system_wrappers/secdert.h:4: error: ‘#pragma’ is not allowed here

It was necessary to add
  #include "seccomon.h"
to file secdert.h
Comment 15 Kai Engert (:kaie) 2007-07-16 23:30:14 PDT
The compilation error was addressed in bug 388120, setting bug back to resolved.

Note You need to log in before you can comment on or make changes to this bug.