Status

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: rob.borcic, Assigned: rob.borcic)

Tracking

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Target is S60 version 3, Symbian 9.x.
(Assignee)

Comment 1

10 years ago
Created attachment 328563 [details] [diff] [review]
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.
(Assignee)

Comment 2

10 years ago
Created attachment 328573 [details] [diff] [review]
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.
Attachment #328563 - Attachment is obsolete: true
(Assignee)

Comment 3

10 years ago
Created attachment 328574 [details] [diff] [review]
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.
(Assignee)

Comment 4

10 years ago
Created attachment 328580 [details] [diff] [review]
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.
(Assignee)

Updated

10 years ago
Attachment #328573 - Flags: review?(stejohns)
(Assignee)

Updated

10 years ago
Attachment #328574 - Flags: review?(stejohns)
(Assignee)

Updated

10 years ago
Attachment #328580 - Flags: review?(stejohns)
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED

Comment 5

10 years ago
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)
(Assignee)

Comment 6

10 years ago
(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

10 years ago
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

10 years ago
+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?
(Assignee)

Comment 9

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

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

(Assignee)

Comment 10

10 years ago
(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

10 years ago
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

10 years ago
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!

(Assignee)

Comment 13

10 years ago
(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

10 years ago
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

10 years ago
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

10 years ago
pushed as changeset:   492:307fb716348c
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 17

10 years ago
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)
Attachment #328573 - Flags: review+

Updated

10 years ago
Attachment #328573 - Flags: review?(stejohns)

Comment 18

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

belatedly marked review+ (fix long since pushed)
Attachment #328574 - Flags: review?(stejohns) → review+

Comment 19

10 years ago
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)
Attachment #328580 - Flags: review?(stejohns) → review+
You need to log in before you can comment on or make changes to this bug.