NSS autoconf build under OS/2

VERIFIED WONTFIX

Status

SeaMonkey
Build Config
P3
normal
VERIFIED WONTFIX
17 years ago
14 years ago

People

(Reporter: jhp (no longer active), Assigned: cls)

Tracking

Trunk
mozilla0.9.3
x86
OS/2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Reporter)

Description

17 years ago
Changes necessary to get NSS autoconf building under OS/2.
(Reporter)

Comment 1

17 years ago
Created attachment 29513 [details] [diff] [review]
first patch
(Reporter)

Comment 2

17 years ago
Added patch with mostly os2 changes, with some of the changes based on 
72693.  Except...

NOTES:
1) Moved NSINSTALL to configure/autoconf.mk, since we use a main 
nsinstall rather than supporting multiple one across components.
2) Rearranged 'compiler-based dependencies' check to be more like that 
in mozilla/configure.in -- COMPILER_DEPENDS was always getting defined 
for me using old check.
Depends on: 72693
(Reporter)

Updated

17 years ago
Blocks: 74461
(Reporter)

Comment 3

17 years ago
Created attachment 29849 [details] [diff] [review]
patch updated with bryner's suggestion
(Reporter)

Comment 4

17 years ago
Updated the patch to include bryner's fix for the --disable-md check 
in configure.in.

Comment 5

17 years ago
Created attachment 31187 [details] [diff] [review]
Just some build stuff and the callback enablement
This bug should probably go to me, since it's the autoconf branch.

Assignee: relyea → bryner

Comment 7

17 years ago
Created attachment 31238 [details] [diff] [review]
Final set of diffs for NSS autoconf

Comment 8

17 years ago
I have these diffs ready to go on NSS_CLIENT_BRANCH. Just 
say the word.
I hate to impose more administrative overhead, but you really need to separate
the NSS patches (security/nss) from the PSM2 patches (security/manager) here. 
Please file a bug on building PSM2 on OS/2 and mark it dependent on this one,
and attach just the PSM2 diffs there.  The PSM2 directory is _not_ branched on
the autoconf branch, so those changes will need a full review from the PSM team.

Thanks.
Status: NEW → ASSIGNED
I don't see why you need to make the callbacks in nsNSSComponent and
nsPKCS12Blob non-members of the classes.  There are existing callback
functions in the tree that are static members of classes.  See
http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeProtocolHandler.cpp
for an example.
Also, should the changes to NSS that aren't in the build system find their way
back to the NSS trunk as well?
Getting the equivalent NSS changes in on the trunk/NSS_CLIENT_BRANCH will be a
prerequisite for the PSM changes as well, since we don't have any definite plans
for turning on the autoconf branch yet.

Comment 13

17 years ago
Anytime you see a class member function (even if it static) passed as a callback 
to a function that expects an extern "C", it is wrong.

These APIs and their typedefs are inside extern "C" so any function passed to 
them MUST be extern "C". Take a look at the build log for SunOS on the ports 
tinderbox page and do a search on extern "C".

I have a list of 1600 instances of this particular problem in Mozilla and I am 
going to work to get them all fixed.

This is essential for real portable code.

As far as separating the changes, I am getting confused as to what is PSM 2.0 
and what is not. Should I only put build specific changes in here?
OK... I read up on the relevant parts of the C++ standard (3.5 and 7.5).  What
makes the pref API callback type extern "C"?  And what makes the linkage on your
static functions "C" rather than "C++" linkage?  Shouldn't you have to declare
them within an |extern "C"| block?  I'm speaking here only from a quick reading
of the standard, so I could easily be wrong...

Comment 15

17 years ago
In the pref case, the code looks like this in prefapi.h:

NSPR_BEGIN_EXTERN_C
.
.
typedef int (*PrefChangedFunc) (const char *, void *);

void PREF_RegisterCallback( const char* domain,
                            PrefChangedFunc callback, void* instance_data );
PrefResult PREF_UnregisterCallback( const char* domain,
                            PrefChangedFunc callback, void* instance_data );
.
.
NSPR_END_EXTERN_C

Note there should probably be a PR_CALLBACK (or come callback) in the typedef.

So this code means that any function that is used as a PrefChangedFunc callback 
has to be defined in an extern "C".

You cannot put extern "C" declarations inside of classes. They have to be moved 
outside of the class and made extern "C", and in most cases you make them 
friends as well because they usually call class functions.

Note by the way that because of the way extern "C" was architected, static and 
extern "C" are mutually exclusive, so the functions end up being externed. This 
is why I made the names look so strange, to guarantee uniqueness.

By the way, SEC_BEGIN_PROTOS and SEC_END_PROTOS correspond to extern "C". In 
this case, I should have probably used the NSPR_EXTERN_C stuff to be more clear.
PSM 2 is everything under security/manager, NSS is everything under
security/coreconf and security/nss.
So there are two definitions of PrefChangedFunc, one in prefapi.h and one in
nsIPref.idl, and only the former is within |extern "C"| and only the latter has
PR_CALLBACK.

Are |static| and |extern "C"| mutually exclusive?  I don't think they should
be according to the C++ spec, and gcc assembler output agrees with that.  (I
think a |static| function in an |extern "C"| block should have internal linkage
with C calling conventions.)  When I looked at gcc3 assembler output for a test
with 4 functions:

extern "C" { static int static_function_in_extern_C() { return 0; } }
static int static_function() { return 0; }
extern "C" { int function_in_extern_C() { return 0; } }
int function() { return 0; }

the third and fourth had ".globl <function-name>" lines (which I assume means
they have external linkage) and the second and fourth had their names mangled
(which seems to be the only difference in the C++ calling convention).

You're right that C calling convention callbacks can't be inside classes -- I
learned that from C++ section 7.5 this morning.

Comment 18

17 years ago
You just have to love stuff like that.

I'll have to think about which one is correct.

As far as static AND extern "C", while logically it should work, and it does 
compile in gcc, it is not correct. 

Unfortunately, the C/C++ architects here decided to overload the meaning of 
extern here and so the concept of static within an extern block doesn't work.

I'd be interested to see what other compiler (particular HPUX and SunOS) do with 
this, but I know if you do this on our compiler, it basically ignores the extern 
"C" and makes them static, creating the same problem.

I'm not sure what the right thing to do is in this case. We probably need to ask 
a C++ guru about that aspect of things.

Incidentally, extern "C" isn't just about calling convention, it also prevents 
name mangling. This is obviously not an issue in the static case.
(Assignee)

Updated

17 years ago
Keywords: mozilla0.9
(Assignee)

Comment 19

17 years ago
Adding resident c++ guru to cc:.

What's the corresponding bug for getting the OS/2 build working on the NSS trunk
or client tag or whatever?  I want to make sure that we have the bare minimal
(preferably zero) non-build changes on the client branch that are not in the NSS
team's tree.  

Comment 20

17 years ago
EMX updates:

security\coreconf\rules.mk
	@cmd /C "$(FILTER) $(SUB_SHLOBJS) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 1,20,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 21,40,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 41,60,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 61,80,$(SUB_SHLOBJS)) >>$@.def"
#
@cmd /C "$(FILTER) $(wordlist 81,100,$(SUB_SHLOBJS)) >>$@.def"

security\nss\configure.in
add this define:
    *-*-os2_emx)
        MKSHLIB = $(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -o $@
(Reporter)

Comment 21

17 years ago
Created attachment 31861 [details] [diff] [review]
NSS only patch
(Reporter)

Comment 22

17 years ago
Removed PSM files to make this bug refer only to NSS autoconf issues.  
The PSM 2.0 files can be found at bug 72693.
(Reporter)

Comment 23

17 years ago
Requires prtypes.h change in bug 76896.
Depends on: 76896

Comment 24

17 years ago
Please add this for EMX:

security\nss\configure.in


    *-*-os2_emx)

        MKSHLIB='$(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -o $@'


(Reporter)

Comment 25

17 years ago
Created attachment 32297 [details] [diff] [review]
new patch with EMX changes
cls, can you take this one?
Assignee: bryner → cls
Status: ASSIGNED → NEW

Comment 27

17 years ago
changing to browser/build config so I can triage and it doesn't show up as an
untargetted bug.
Component: Libraries → Build Config
Keywords: mozilla0.9
Priority: -- → P3
Product: NSS → Browser
Target Milestone: --- → mozilla0.9.3
Version: unspecified → other
(Assignee)

Comment 28

17 years ago
I think we've given up on the nss autoconf branch for now.  Marking WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 29

17 years ago
verified
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.