298674 - nspr support for RISC OS

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement)

Description

[Peter Naulls]

User-Agent:       Mozilla/1.10 [en] (Compatible; RISC OS 4.39; Oregano 1.10)
Build Identifier: 

This patch and files add support for RISC OS to nspr.  This was used in the 
port of Firefox to RISC OS.

Reproducible: Always

Steps to Reproduce:

1.
2.
3.

Comment 1

[Peter Naulls]

Attachment: RISC OS nspr support

Also contains some spelling corrections.

Comment 2

[Peter Naulls]

Attachment: cfg file for nspr RISC OS support

This file belongs in nsprpub/pr/include/md/

Comment 3

[Peter Naulls]

Attachment: include file for nspr RISC OS support

Comment 4

[Peter Naulls]

Attachment: RISC OS nspr unix support

This file belongs in nsprpub/src/md/unix/riscos.c

Comment 5

[Wan-Teh Chang]

Comment on attachment 187194 [details] [diff] [review]
RISC OS nspr support

>Index: configure.in
...
>+*-riscos*)
>+    OS_TARGET=RISCOS
>+    OS_ARCH=RISCOS
>+    AC_DEFINE(XP_UNIX)
>+    AC_DEFINE(RISCOS)
>+    AC_DEFINE(_PR_NEED_H_ERRNO)
>+    USE_PTHREADS=1
>+    MDCPUCFG_H=_riscos.cfg
>+    PR_MD_CSRCS=riscos.c
>+    DLL_SUFFIX=a
>+    LD="/home/riscos/env/ro-ar cr"
>+    ;;

The setting of OS_TARGET and OS_ARCH should not be done here.
It should be done near the beginning of this file.  Search for
the first occurrence of "IRIX64" in this file.	That's where
you modify OS_ARCH if the default value is not suitable.
(By default OS_TARGET equals OS_ARCH, so in your case you
don't need to set OS_TARGET.)

>Index: pr/include/md/_pth.h
...
> #elif defined(HPUX) || defined(LINUX) || defined(SOLARIS) \
> 	|| defined(FREEBSD) || defined(NETBSD) || defined(OPENBSD) \
> 	|| defined(BSDI) || defined(NTO) || defined(DARWIN) \
>-	|| defined(UNIXWARE)
>+	|| defined(UNIXWARE) || defined(RISCOS)
> #define _PT_PTHREAD_YIELD()            	sched_yield()

Since you define sched_yield as pthread_yield in _riscos.h, this
seems wrong.  It is better to just define _PT_PTHREAD_YIELD()
as pthread_yield() in this file and remove the sched_yield macro
definition in _riscos.h.

>Index: pr/src/linking/prlink.c
...
>-#if !defined(XP_MAC) && !defined(XP_BEOS)
>+#if !defined(XP_MAC) && !defined(XP_BEOS) && !defined(RISCOS)
>     PR_LOG(_pr_linker_lm, PR_LOG_MIN, ("Loaded library %s (init)", lm?lm->name:"NULL"));
> #endif

Why do you need this change?

>Index: pr/src/malloc/prmem.c
...
>-#elif defined(USE_MACH_DYLD)
>+#elif defined(USE_MACH_DYLD) || defined(RISCOS)
> 
> static void *
> pr_FindSymbolInProg(const char *name)
> {
>     /* FIXME: not implemented */
>     return NULL;
> }

I will implement a better fix that tests !defined(HAVE_DLL)
instead.

>Index: pr/src/misc/prsystem.c
...
>+#elif defined(RISCOS)
>+    numCpus = 1;

Is this guaranteed to be 1, or it is an expedient way to get
the RISC OS port done?

Comment 6

[Wan-Teh Chang]

Comment on attachment 187195 [details]
cfg file for nspr RISC OS support

>#define PR_ALIGN_OF_SHORT   1
>#define PR_ALIGN_OF_INT     1
>#define PR_ALIGN_OF_LONG    1
>#define PR_ALIGN_OF_INT64   1
>#define PR_ALIGN_OF_FLOAT   1
>#define PR_ALIGN_OF_DOUBLE  1
>#define PR_ALIGN_OF_POINTER 1
>#define PR_ALIGN_OF_WORD    1

These values don't look right.

The best way to construct this file is to begin with
the .cfg file for some other platform.	Then, compile
and run mozilla/nsprpub/pr/include/gencfg.c.  You may
need to make some modifications to gencfg.c in order
to compile it on RISC OS.  Finally, use the output of
the gencfg.c program in _riscos.cfg.  The output is only
a subset of the macro definitions in _riscos.cfg.

Comment 7

[Wan-Teh Chang]

Comment on attachment 187197 [details]
include file for nspr RISC OS support

>#define sched_yield pthread_yield

This should be removed.  (See my comment 5.)

>#define uint unsigned int
>
>#undef atoll

Why do yo need these two?

>#include "primpl.h"

This should be removed.

Comment 8

[Wan-Teh Chang]

Attachment: Initial checkin of RISC OS port contributed by Peter Naulls

I checked in the subset of Peter Naulls's patches that
I reviewed and approved, with some rewrites.  Here is
the patch file for what I checked in on the NSPR tip.

Peter, for your work on this bug, please use the
NSPR tip:
    cvs co -A mozilla/nsprpub

Comment 9

[Wan-Teh Chang]

Comment on attachment 187984 [details] [diff] [review]
Initial checkin of RISC OS port contributed by  Peter Naulls

Peter, I forgot to mention that I've already checked in
your spelling error fixes separately, and that I used
the new MPL/GPL/LGPL in the three new files you added.

Comment 10

[Peter Naulls]

> Since you define sched_yield as pthread_yield in _riscos.h, this
> seems wrong.  It is better to just define _PT_PTHREAD_YIELD()
> as pthread_yield() in this file and remove the sched_yield macro
> definition in _riscos.h.

I have done this.

> >Index: pr/src/linking/prlink.c
> ...
> >-#if !defined(XP_MAC) && !defined(XP_BEOS)
> >+#if !defined(XP_MAC) && !defined(XP_BEOS) && !defined(RISCOS)
> >     PR_LOG(_pr_linker_lm, PR_LOG_MIN, ("Loaded library %s (init)", 
lm?lm->name:"NULL"));
> > #endif
> 
> Why do you need this change?

This was because of static linking, but is no longer relevant because of
changes in nspr

> >+#elif defined(RISCOS)
> >+    numCpus = 1;
> 
> Is this guaranteed to be 1, or it is an expedient way to get
> the RISC OS port done?

This is always 1.

> >#define uint unsigned int
> >
> >#undef atoll
> 
> Why do yo need these two?

I believe these were work arounds for problems in our C library which I
have since fixed.

I will attach the final changes for RISC OS nspr support.

Comment 11

[Peter Naulls]

Attachment: Additional nspr support for RISC OS

Additional nspr support based upon above comments.  There's also a missing
brace which this adds.

Comment 12

[Wan-Teh Chang]

Comment on attachment 190007 [details] [diff] [review]
Additional nspr support for RISC OS

r=wtc.	I've checked in this patch on the NSPR tip.
Peter, please verify that the NSPR tip works on RISC OS
now. (cvs co -A mozilla/nsprpub)

You can use mozilla/nsprpub/pr/tests/runtests.sh to
run our test suite.  (Need to "make" the test programs
in nsprpub/pr/tests first.)

Thank you for catching the missing closing parenthesis
in prmem.c.

Your _pth.h change uses sched_yield().	Does this mean
RISC OS have both sched_yield() and pthread_yield()?
(sched_yield() is better because it is the standard POSIX
function.)

Comment 13

[Peter Naulls]

> (From update of attachment 190007 [details] [diff] [review])
> r=wtc. I've checked in this patch on the NSPR tip.
> Peter, please verify that the NSPR tip works on RISC OS
> now. (cvs co -A mozilla/nsprpub)

It works just fine on RISC OS, and indeed, on Linux where I'm cross
compiling, and indeed I used just such a tree to produce the patches.
However, you have mentioned at all why such a checkout is so great,
since it produces precisely the same subtree that you get from checking
out Mozilla head.

> You can use mozilla/nsprpub/pr/tests/runtests.sh to
> run our test suite.  (Need to "make" the test programs
> in nsprpub/pr/tests first.)

I'm well aware of the tests of course.  Some of them don't make sense on
RISC OS.

> Your _pth.h change uses sched_yield(). Does this mean
> RISC OS have both sched_yield() and pthread_yield()?
> (sched_yield() is better because it is the standard POSIX
> function.)

It is not RISC OS, but the C library that we're using.  It only has
pthread_yield.  I could add the sched version, but it would do precisely
the same thing.  

Comment 14

[Wan-Teh Chang]

Peter: I don't quite understand your question about source
trees.  I'll try to answer it below.

Mozilla/Firefox checks out NSPR with a CVS branch tag
called NSPRPUB_PRE_4_2_CLIENT_BRANCH.  Mozilla/Firefox
doesn't check out the HEAD of NSPR.

"cvs co -A" is the same as "cvs co -r HEAD".

I've only checked in your RISC OS support code on the
HEAD of NSPR.  I will move it to NSPRPUB_PRE_4_2_CLIENT_BRANCH
after testing.

I am confused as to why we can define _PT_PTHREAD_YIELD()
as sched_yield().  If your C library only has pthread_yield(),
where is sched_yield() defined?

Comment 15

[Peter Naulls]

For the moment, I have no further changes to this and am happy with the current
code in nspr for RISC OS, and the bug can probably be closed.   

In the near future, I may have some tidying for nspr as well as some other
changes for the main Mozilla code to add full RISC OS support.  Thanks.

Comment 16

[Wan-Teh Chang]

Marked the bug fixed in NSPR 4.6.1.