437293 - port to Symbian

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tracing Virtual Machine, defect)

Description

[Rob Borcic]

Target is S60 version 3, Symbian 9.x.

Comment 1

[Rob Borcic]

Attachment: Symbian shell and porting layer, plus proxy

This is the Symbian version of the avmplus shell plus a windows based proxy that communicated with a TestShell wrapper application on the device. This patch also includes a partial porting layer.

Comment 2

[Rob Borcic]

Attachment: Replacement for previous patch -- this one includes GPL header in all files.

Same as previous patch, except now all new files have the GPL header.

Comment 3

[Rob Borcic]

Attachment: MMgc changes required for Symbian/ARM port.

Changes to allow MMgc to use a porting API plus an implementation of the porting API for Symbian/ARM.

Comment 4

[Rob Borcic]

Attachment: Changes required for porting layer and proper operation on Symbian/ARM

This patch makes some minor modifications to work with the new porting layer. It also adds additional support for Symbian/ARM.

Comment 5

[Steven Johnson]

I'm a little concerned about the PORTING_API stuff... if this is the approach we need to do a credible Symbian port, fine, but then we should really adapt all the other platforms to this model too, which means we should probably have more people take a look at it to see if it fits elsewhere.

(adding a few people to cc who may want to comment... not sure who from Mozilla to add, adding Ben Smedberg for now)

Comment 6

[Rob Borcic]

(In reply to comment #5)
> I'm a little concerned about the PORTING_API stuff... if this is the approach
> we need to do a credible Symbian port, fine, but then we should really adapt
> all the other platforms to this model too, which means we should probably have
> more people take a look at it to see if it fits elsewhere.

This approach is just one of may that could be adopted. The current code is difficult to port because there are multiple places across multiple files that need to be touched. This approach tries to centralize the platform-dependent stuff into two places -- one for MMgc and one for the rest of the vm. Ideally, the #ifdef XXX_PORTING_API lines could be replaced with just the porting API macros and each platform would have it's own definitions for these macros and porting to a new platform would just require those definitions to be ported.

Comment 7

[Edwin Smith]

since most of the #ifdef PORTING_API lines are just a single if-else (one using the api, the other doing what we had before), could we go ahead and define the macros?  initially all platforms except symbian will just have the same macro definitions, and we can refine over time.

Comment 8

[Steven Johnson]

+1 — I don’t want to hold things up, just want to be sure that other folks have peeped at the porting approach and think it looks reasonable.

Rob, would it be too much to go ahead and define the macros everywhere else?

For that matter, should we go ahead and add the portapi.h file to every platform as well? Or should we just open bugs for that?

Comment 9

[Rob Borcic]

A description of the porting layer has been posted to the wiki:

http://wiki.mozilla.org/Tamarin:Porting

Comment 10

[Rob Borcic]

(In reply to comment #8)
> Rob, would it be too much to go ahead and define the macros everywhere else?
> 
> For that matter, should we go ahead and add the portapi.h file to every
> platform as well? Or should we just open bugs for that?

My preference would be to open bugs for the other platforms and leave the ifdefs for PORTING_API in place until that work is complete. I don't have the ability to test changes on all platforms and would hate to wreak havoc.

Comment 11

[Benjamin Smedberg]

What amount of this *has* to be in macros? Can we use C++ inline functions (from platform-specific headers) for some/all of this and avoid excess macrology?

Comment 12

[Edwin Smith]

There's no ideology at play, we can be practical about it.  macros are a tool to keep the C/C++ code cleaner than littering #ifdefs around.  i would imagine in many cases it'll be a macro that calls a non-inline function.

you can't #undef and #define an inline function; this could be useful if all the macros are defined in a generic way and then platforms can tweak just the ones they need to.  obviously there's other ways to do this - comments to the wiki!

Comment 13

[Rob Borcic]

(In reply to comment #12)
> There's no ideology at play, we can be practical about it.  macros are a tool
> to keep the C/C++ code cleaner than littering #ifdefs around.  i would imagine
> in many cases it'll be a macro that calls a non-inline function.
> 
> you can't #undef and #define an inline function; this could be useful if all
> the macros are defined in a generic way and then platforms can tweak just the
> ones they need to.  obviously there's other ways to do this - comments to the
> wiki!

I personally dislike macros, but they are more consistent than inline functions across platforms  and compilers. The inline keyword is not always respected. 

And in support of Edwin's point, it would be easier to define a single file that contains the posix/stdlib functions and undef and redefine specific ones in the platform's specific file.

Comment 14

[Steven Johnson]

At least one compiler used for TC (SunPro) has no directive to force something inline. I agree that macros are suboptimal but with careful usage I think we can mitigate the risk.

Comment 15

[Steven Johnson]

Looks like there are no strong objections -- I'm going to push this as-is, and Rob is going to enter bugs to adapt all other current platforms to use this approach.

Comment 16

[Steven Johnson]

pushed as changeset:   492:307fb716348c

Comment 17

[Steven Johnson]

Comment on attachment 328573 [details] [diff] [review]
Replacement for previous patch -- this one includes GPL header in all files.

belatedly marked review+ (fix long since pushed)

Comment 18

[Steven Johnson]

Comment on attachment 328574 [details] [diff] [review]
MMgc changes required for Symbian/ARM port.

belatedly marked review+ (fix long since pushed)

Comment 19

[Steven Johnson]

Comment on attachment 328580 [details] [diff] [review]
Changes required for porting layer and proper operation on Symbian/ARM

belatedly marked review+ (fix long since pushed)