476134 - [OS/2] Fix plugin code to allow building with GCC 4.3.x

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Peter Weilbacher]

Follow-up from bug 476029 comment 2.
I guess this will also eliminate the last hurdles for bug 454956.

Comment 1

[Rich Walsh]

Attachment: 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 2

[Peter Weilbacher]

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 3

[Josh Aas]

Comment on attachment 359915 [details] [diff] [review]
patch to npfunctions.h

Please don't check this in until I have looked at it.

Comment 4

[Josh Aas]

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?

Comment 5

[Peter Weilbacher]

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?

Comment 6

[Josh Aas]

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.

Comment 7

[Josh Aas]

And thanks for the explanation, I'll review again soon.

Comment 8

[Peter Weilbacher]

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.

Comment 9

[Josh Aas]

Attachment: fix v1.0

Same thing, updated to current trunk.

Comment 10

[Peter Weilbacher]

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.

Comment 11

[Peter Weilbacher]

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.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 360656 [details] [diff] [review]
fix v1.0

a191=beltzner

Comment 13

[Peter Weilbacher]

Fixed in mozilla-1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a897e9525f7e