Follow-up from bug 476029 comment 2. I guess this will also eliminate the last hurdles for bug 454956.
Created attachment 359915 [details] [diff] [review] patch to npfunctions.h This patches modules/plugin/base/public/npfunctions.h to eliminate calling convention errors in modules/plugin/default/os2/npos2.cpp. The NPP_* functions in nsapi.h are all declared to be NP_LOADDS, which on OS/2 resolves to '_System'. However, the typedefs for these functions lack this declaration, producing pointer conversion errors when compiling.
Comment on attachment 359915 [details] [diff] [review] patch to npfunctions.h Makes sense to me, but those plugin files should get sr, too, even though it doesn't change anything for the other platforms.
Comment on attachment 359915 [details] [diff] [review] patch to npfunctions.h Please don't check this in until I have looked at it.
Attachment #359915 - Flags: review?(joshmoz)
Is there really no way to change the calling convention on the other end so we don't have to do this to npfunctions.h? What is nsapi.h?
Sorry, Josh. Didn't want to cut you out. "nsapi.h" in comment 1 obviously was a typo and was supposed to read "npapi.h". We have to use _System calling convention which is the standard way to use all external functions on OS/2 and has to be maintained especially for the plugins for backward compatibility. As that convention is used in npapi.h we have to adapt npfunction.h to follow because GCC 4.3 is too picky to let it go otherwise. With that compiler we now get weird errors like <path>/modules/plugin/default/os2/npos2.cpp: In function 'NPError NP_GetEntryPoints(NPPluginFuncs*)': <path>/modules/plugin/default/os2/npos2.cpp:83: error: invalid conversion from 'NPError (*)(char*, NPP_t*, uint16_t, int16_t, char**, char**, NPSavedData*)' to 'NPError(*)(char*, NPP_t*, uint16_t, int16_t, char**, char**, NPSavedData*)' <path>/modules/plugin/default/os2/npos2.cpp:84: error: invalid conversion from 'NPError (*)(NPP_t*, NPSavedData**)' to 'NPError (*)(NPP_t*, NPSavedData**)' <path>/modules/plugin/default/os2/npos2.cpp:85: error: invalid conversion from 'NPError (*)(NPP_t*, NPWindow*)' to 'NPError (*)(NPP_t*, NPWindow*)' <path>/modules/plugin/default/os2/npos2.cpp:86: error: invalid conversion from 'NPError (*)(NPP_t*, char*, NPStream*, NPBool, uint16_t*)' to 'NPError (*)(NPP_t*, char*, NPStream*, NPBool, uint16_t*)' (Sorry should have been in comment 0.) GCC 3.3.5, that we were using before, let this pass and apparently did the right thing. Why do you see changing npfunctions.h as a problem? It's just an internal file, right?
Changing npfunctions.h in that way is not technically a problem, but I like to keep it as uncluttered as possible. npfunctions.h is not simply internal - that, plus "npapi.h", "nptypes.h", and "npruntime.h" are the standard NPAPI headers. All plugins should be able to compile against them and run in all browsers that support NPAPI. The files should match between browser implementations but they don't, we're working on it.
And thanks for the explanation, I'll review again soon.
Thanks, Josh. I had forgotten that you had renamed npupp.h to npfunctions.h and got confused because I didn't find the new name mentioned anywhere on MDC.
Attachment #359915 - Flags: superreview?(jst) → superreview+
Created attachment 360656 [details] [diff] [review] fix v1.0 Same thing, updated to current trunk.
Attachment #359915 - Attachment is obsolete: true
Thanks Rich and Josh! Pushed to trunk: http://hg.mozilla.org/mozilla-central/rev/4fad8c29c9be I guess we will need this for FF 3.1, too, to eventually be able to produce a patch-free OS/2 build. But let's wait a while before asking for approval.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Comment on attachment 360656 [details] [diff] [review] fix v1.0 We need this to build the 1.9.1 branch with a current compiler. This hasn't caused any problems and is essentially an OS/2 only change.
Attachment #360656 - Flags: approval1.9.1?
Attachment #360656 - Flags: approval1.9.1? → approval1.9.1+
Fixed in mozilla-1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a897e9525f7e
You need to log in before you can comment on or make changes to this bug.