Closed Bug 33610 Opened 25 years ago Closed 25 years ago

PR_CALLBACK and PR_CALLBACK_PTR

Categories

(NSPR :: NSPR, defect, P3)

Other
Other
defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: mkaply, Assigned: larryh)

Details

Chris Blizzard told me to go ahead and open this as a bug: We have an interesting problem on the OS/2 platform. Take the following function declaration: static PR_CALLBACK PLHashNumber _hashValue(const void *key) on all platforms but Win16, PR_CALLBACK is not defined. On Win16 it is used for a linkage. For Win16, this linkage goes before the return type. However, on OS/2, we have to specify the linkage between the return type and the function like this: static PLHashNumber PR_CALLBACK _hashValue(const void *key) PR_CALLBACK is defined to _Optlink, our linkage. To fix this problem, we must add #ifdefs to all functions that use PR_CALLBACK. On 4.X, a solution was created for this problem but apparently not ported to Mozilla. The solution is to use a macro like this: For Win16: #define PR_CALLBACK_PTR(__x) PR_CALLBACK __x For OS/2: #define PR_CALLBACK_PTR(__x) __x PR_CALLBACK For all other platforms: #define PR_CALLBACK_PTR(__x) PR_CALLBACK __x Then the above function becomes: static PR_CALLBACK_PTR(PLHashNumber) _hashValue(const void *key) and does not have to be #ifdeffed for any platforms. We would like to suggest that this change be made to prtypes.h Another option would be to define something like PR_LINKAGE like this: static PR_CALLBACK PLHashNumber PR_LINKAGE _hashValue(const void *key) Where PR_LINKAGE only gets set on OS/2. In addition, we have found a number of areas throughtout Mozilla that are NSPR callbacks but lack the PR_CALLBACK convention. We assume this is because WIN16 is not needed for Mozilla.
For some reason, PR_CALLBACK was originally defined without the return type as a parameter; it should have been defined similar to PR_EXPORT, etc. For static callback functions on OS2 (XP_OS2_VACPP), PR_STATIC_CALLBACK(x) is defined correctly in prtypes.h #define PR_STATIC_CALLBACK(__x) static __x _Optlink Reassigning to wtc.
Assignee: srinivas → wtc
Michael, can you use the existing PR_STATIC_CALLBACK macro and declare your hash function like this? PR_STATIC_CALLBACK(PLHashNumber) _hashValue(const void *key); Larry, on Win16, can you put PR_CALLBACK *after* the return type?
Assignee: wtc → larryh
The code in question is: http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsHashtable.cpp#31 Looking at this code in particular, it probably should be a PR_STATIC_CALLBACK given that it is a static function, but we have found other places that are not static. Would you prefer changing stuff from PR_CALLBACK to PR_STATIC_CALLBACK or from PR_CALLBACK to PR_CALLBACK_PTR, given that we are going to have to make changes anyway. And I guess the other issue is how to handle this right now as this is a rather large issue. I am researching to find out how many NSPR specific changes there are. Maybe most of this should go in as simple bugs against components. Should the above code be PR_STATIC_CALLBACK?
It would appear that the browser code using PR_CALLBACK is busted. Here's the scoop: The syntax defined for PR_CALLBACK is: <scope> <type> PR_CALLBACK <functionName>(<argList>) This has always been the syntax for using PR_CALLBACK. Currently, PR_CALLBACK expands to nothing on all platforms except Win16. Recall that Win16 is no longer supported and nobody builds things for it anymore, so any inappropriate use of PR_CALLBACK would go unnoticed. So, of course it works on all supported platforms. ... But OS/2, which requires that PR_CALLBACK expand to something that OS/2 compilers need. Solution: Change the code using PR_CALLBACK to usse the correct syntax.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → INVALID
So a few questions then: 1. Doesn't Win16 still need to be built for NSPR? Don't they want to keep having WIN16 stuff? 2. Would WIN16 support PR_CALLBACK in the "correct" place? 3. Who would be reponsible for making these kind of changes?
1. NSPR has not built for Win16 in many moons; last I recall trying to build NSPR for Win16 (3Q 1998), it had quit building. There is no requirement for building a Win16 Mozilla client, nor commercial client. 2. Yes. Last I looked. The test cases in mozilla/nsprpub/pr/tests/*.c that use PR_CALLBACK, compiled and ran last I built and ran them. 3. Who? Probably those own the parts with the inappropriate use of PR_CALLBACK, or someone who wants it to build and work for OS/2.
OK, so I am still confused here. It seems like we have a couple different problems: Improper use of static PR_CALLBACK when it should be STATIC_PR_CALLBACK Improper location of PR_CALLBACK PR_CALLBACK_PTR possibly should be used. I guess what I am trying to figure out is what is the right thing to do. Given the other STATIC_PR_CALLBACK_PTR(xxx) macros, I would think that PR_CALLBACK_PTR(xxx) would be the right thing. My hope is that we could get this fixed the right way in one try so if anyone else runs into this, we could fix it. If someone tells me exactly what to do, I would like to fix it and I would like to have this bug open to put in the code for reviews and for checkin. Thanks
You need to log in before you can comment on or make changes to this bug.