277954 - Port JS to WINCE

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Doug Turner (:dougt)]

Comment 1

[Doug Turner (:dougt)]

Attachment: Patch v.1

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 170940 [details] [diff] [review]
Patch v.1

couple things i don't really like.

wince doesn't have localeconv().  I can add this functionality to my shim
layer, but not sure if other platforms would need a default in the js code.

in prmjtime, GetSystemTimeAsFileTime is a helper function
GetSystemTime+SystemTimeToFileTime

Oh, and I just noticed that the changes to Makefile.in are a bit off.  I messed
up the non WINCE target for jscpucfg.  New patch to correct that coming up.

Comment 3

[Doug Turner (:dougt)]

Attachment: Patch v.2

fixes the Makefile.in wince target for jscpucfg

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 170941 [details] [diff] [review]
Patch v.2

>+ifeq ($(OS_ARCH),WINCE)
>+jscpucfg$(HOST_BIN_SUFFIX): jscpucfg.c Makefile.in
>+	$(HOST_CC) -o $@ $(HOST_CFLAGS) $(DEFINES) $(NSPR_CFLAGS)  $<
>+else
> jscpucfg$(HOST_BIN_SUFFIX): jscpucfg.c Makefile.in
> 	$(HOST_CC) $(HOST_CFLAGS) $(DEFINES) $(NSPR_CFLAGS) $(OUTOPTION)$@ $<
> endif

Is $(OUTOPTION) not usable on WINCE builds?  It seems to work for all others. 
If the problem is that it needs to come first, just move it to right after
$(HOST_CC) and avoid the ifeq/else/endif.

>+++ src/jslibmath.h	11 Jan 2005 18:09:56 -0000
>@@ -112,21 +112,21 @@
>  * Use math routines in fdlibm.
>  */
> 
> #undef __P
> #ifdef __STDC__
> #define __P(p)  p
> #else
> #define __P(p)  ()
> #endif
> 
>-#if defined _WIN32 || defined SUNOS4
>+#if defined _WIN32 || defined SUNOS4 && !defined WINCE

&& binds more tihgtly than ||, so that's one bug.  But just parenthesizing the
existing || expr is silly, since in the SUNOS4 case there's no point in
checking !WINCE.  So this ought to be #if (defined _WIN32 && !defined WINCE) ||
defined SUNOS4.  Right?

>+++ src/jslong.h	11 Jan 2005 18:09:56 -0000
>@@ -76,20 +76,21 @@
> #define JSLL_MININT   JSLL_MinInt()
> #define JSLL_ZERO     JSLL_Zero()
> 
> #ifdef JS_HAVE_LONG_LONG
> 
> #if JS_BYTES_PER_LONG == 8
> #define JSLL_INIT(hi, lo)  ((hi ## L << 32) + lo ## L)
> #elif (defined(WIN32) || defined(WIN16)) && !defined(__GNUC__)
> #define JSLL_INIT(hi, lo)  ((hi ## i64 << 32) + lo ## i64)
> #else
>+#define JSLL_INIT(hi, lo)  ((hi ## i64 << 32) + lo ## i64)
> #define JSLL_INIT(hi, lo)  ((hi ## LL << 32) + lo ## LL)
> #endif

Copy/paste bug?  Clearly wrong.

> #if (defined XP_WIN || defined XP_OS2) &&                                     \
>+    !defined WINCE &&                                                         \

So XP_WIN is defined for WINCE, but not XP_WIN32?  What else is defined, if any
standard macro?

>+#ifndef WINCE
>     locale = localeconv();
>     rt->thousandsSeparator =
>         JS_strdup(cx, locale->thousands_sep ? locale->thousands_sep : "'");
>     rt->decimalSeparator =
>         JS_strdup(cx, locale->decimal_point ? locale->decimal_point : ".");
>     rt->numGrouping =
>         JS_strdup(cx, locale->grouping ? locale->grouping : "\3\0");
>+#else
>+    rt->thousandsSeparator = JS_strdup(cx, "'");
>+    rt->decimalSeparator   = JS_strdup(cx, ".");
>+    rt->numGrouping        = JS_strdup(cx, "\3\0");
>+#endif
> 
>     return rt->thousandsSeparator && rt->decimalSeparator && rt->numGrouping;

On WINCE, how does one convert a number to a localized currency string?  There
must be a way.

> JS_PUBLIC_API(void) JS_Assert(const char *s, const char *file, JSIntn ln)
> {
> #ifdef XP_MAC
>     dprintf("Assertion failure: %s, at %s:%d\n", s, file, ln);
> #else
>     fprintf(stderr, "Assertion failure: %s, at %s:%d\n", s, file, ln);
> #endif
> #if defined(WIN32)
>+#if !defined(WINCE)
>     DebugBreak();
> #endif
>+#endif

So both WIN32 and WINCE are defined on WINCE?  Hard to believe.  Don't nest
#ifs here, using && if you must.

> #ifdef XP_WIN
>+#ifndef WINCE 

Why is this here, if you changed what follows to work on WINCE and WIN32?  You
need to test on both before attaching another patch :-/.

>     /* The windows epoch is around 1600. The unix epoch is around 1970.
>        win2un is the difference (in windows time units which are 10 times
>        more precise than the JS time unit) */
>-    GetSystemTimeAsFileTime(&time);
>+    SYSTEMTIME st;
>+    GetSystemTime(&st);
>+    SystemTimeToFileTime(&st,&time);
>+
>     /* Win9x gets confused at midnight
>        http://support.microsoft.com/default.aspx?scid=KB;en-us;q224423
>        So if the low part (precision <8mins) is 0 then we get the time
>        again. */
>     if (!time.dwLowDateTime) {
>-        GetSystemTimeAsFileTime(&midnight);
>+        GetSystemTime(&st);
>+        SystemTimeToFileTime(&st,&midnight);
>         time.dwHighDateTime = midnight.dwHighDateTime;
>     }
>     JSLL_UI2L(s, time.dwHighDateTime);
>     JSLL_UI2L(us, time.dwLowDateTime);
>     JSLL_SHL(s, s, 32);
>     JSLL_ADD(s, s, us);
>     JSLL_SUB(s, s, win2un);
>     JSLL_DIV(s, s, ten);
>     return s;
> #endif
>+#endif
> 
> #if defined(XP_UNIX) || defined(XP_BEOS)
> #ifdef _SVID_GETTOD   /* Defined only on Solaris, see Solaris <sys/types.h> */
>     gettimeofday(&tv);
> #else
>     gettimeofday(&tv, 0);
> #endif /* _SVID_GETTOD */
>     JSLL_UI2L(s2us, PRMJ_USEC_PER_SEC);
>     JSLL_UI2L(s, tv.tv_sec);
>     JSLL_UI2L(us, tv.tv_usec);

My gmail account is for bug-watching, not reviewing -- please use
[e-mail].

/be

Comment 5

[Benjamin Smedberg]

On WinCE, We should be defining XP_WIN but not XP_WIN32. If that's not the case
we should fix configure.

Comment 6

[Doug Turner (:dougt)]

brendan, OUTOPTION is set to -Fo for WINCE.  However, it seams like -o is needed
for HOST CC.

The windows defines are:

XP_WIN32 
XP_WIN 
WIN32 
_WIN32
WINCE

All are defined since we are using a shim to replace missing WINCE functionality.

Not sure I understand why i don't want XP_WIN32. Benjamin?

Looking into the localized currency stuff; I will remove this from the patch and
implement localconv in the shim.

New patch shortly.

Comment 7

[Doug Turner (:dougt)]

Attachment: Patch v.3

This is a much cleaner patch.  I was able to shimmy the localeconv stuff.  The
deal there was that localeconv is defined in a header, but in the c library for
arm4, it isn't.  Basically, you have to roll your own.	I can do this outside
of JS-land, of course.

Comment 8

[Doug Turner (:dougt)]

Attachment: patch v.4

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 172731 [details] [diff] [review]
patch v.4

>@@ -375,7 +375,12 @@
> 	$(HOST_CC) $(HOST_CFLAGS) $(DEFINES) -o $@ $<
> endif
> else
>+ifeq ($(OS_ARCH),WINCE)
>+jscpucfg$(HOST_BIN_SUFFIX): jscpucfg.c Makefile.in
>+	$(HOST_CC) -o $@ $(HOST_CFLAGS) $(DEFINES) $(NSPR_CFLAGS)  $<
>+else
> jscpucfg$(HOST_BIN_SUFFIX): jscpucfg.c Makefile.in
> 	$(HOST_CC) $(HOST_CFLAGS) $(DEFINES) $(NSPR_CFLAGS) $(OUTOPTION)$@ $<

My point was that for cross-compiling, we should have a HOST_OUTOPTION that you
can use to avoid the ifeq ($(OS_ARCH),WINCE), to avoid restating the
jscpucfg$(HOST_BIN_SUFFIX) rule with only the -o vs. $(OUTOPTION) difference. 
Maybe a separate bug, or could you roll that change into this patch?

>+++ js/src/jstypes.h	28 Jan 2005 19:58:48 -0000
>@@ -221,7 +221,7 @@
> #define JS_MIN(x,y)     ((x)<(y)?(x):(y))
> #define JS_MAX(x,y)     ((x)>(y)?(x):(y))
> 
>-#if (defined(XP_MAC) || defined(XP_WIN)) && !defined(CROSS_COMPILE)
>+#if (defined(XP_MAC) || (defined(XP_WIN) && !defined(WINCE))) && !defined(CROSS_COMPILE)

I missed this last time -- why do you need the !defined(WINCE) here?  Oh, so
you always use jsautocfg.h for WINCE, whether cross-compiling or not?  This
#if/#elif is pretty ugly, maybe it ought to be redone to make the conditions
simpler, at the cost of duplicating the #includes a little across separate
#if/#elif "then" clauses?

/be

Comment 10

[Frank Wein [:mcsmurf]]

Comment on attachment 170940 [details] [diff] [review]
Patch v.1

canceling obsolete review request

Comment 11

[Doug Turner (:dougt)]

This landed already.  Sorry that I didn't include a cvs commit log; bonsai knows
all.

Comment 12

[Doug Turner (:dougt)]

Comment on attachment 170959 [details] [diff] [review]
Patch v.3

clearing request flag on fixed bug.

Comment 13

[Doug Turner (:dougt)]

Comment on attachment 172731 [details] [diff] [review]
patch v.4

clearing request flag on fixed bug.