Closed
Bug 33610
Opened 25 years ago
Closed 25 years ago
PR_CALLBACK and PR_CALLBACK_PTR
Categories
(NSPR :: NSPR, defect, P3)
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
Comment 2•25 years ago
|
||
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
Reporter | ||
Comment 3•25 years ago
|
||
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?
Assignee | ||
Comment 4•25 years ago
|
||
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
Reporter | ||
Comment 5•25 years ago
|
||
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?
Assignee | ||
Comment 6•25 years ago
|
||
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.
Reporter | ||
Comment 7•25 years ago
|
||
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.
Description
•