Closed Bug 74459 Opened 24 years ago Closed 23 years ago

NSS autoconf build under OS/2

Categories

(SeaMonkey :: Build Config, defect, P3)

x86
OS/2
defect

Tracking

(Not tracked)

VERIFIED WONTFIX
mozilla0.9.3

People

(Reporter: jhpedemonte, Assigned: cls)

References

Details

Attachments

(6 files)

Changes necessary to get NSS autoconf building under OS/2.
Attached patch first patch (deleted) — Splinter Review
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
Blocks: 74461
Updated the patch to include bryner's fix for the --disable-md check in configure.in.
This bug should probably go to me, since it's the autoconf branch.
Assignee: relyea → bryner
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.
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...
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.
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.
Keywords: mozilla0.9
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.
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 $@
Attached patch NSS only patch (deleted) — Splinter Review
Removed PSM files to make this bug refer only to NSS autoconf issues. The PSM 2.0 files can be found at bug 72693.
Requires prtypes.h change in bug 76896.
Depends on: 76896
Please add this for EMX: security\nss\configure.in *-*-os2_emx) MKSHLIB='$(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -o $@'
Attached patch new patch with EMX changes (deleted) — Splinter Review
cls, can you take this one?
Assignee: bryner → cls
Status: ASSIGNED → NEW
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
I think we've given up on the nss autoconf branch for now. Marking WONTFIX.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WONTFIX
verified
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: