Closed Bug 418665 Opened 14 years ago Closed 14 years ago

windows mobile build error in js/src/jsdate.c

Categories

(Core :: JavaScript Engine, defect)

All
Windows Mobile 6 Standard
defect
Not set
blocker

Tracking

()

VERIFIED FIXED

People

(Reporter: dougt, Assigned: dougt)

Details

Attachments

(1 file, 2 obsolete files)

GetLocalTime is defined in the Windows Mobile SDK and this conflicts with the static function defined in js/src/jsdate.c.

I worked around this by something like this:

+#ifdef WINCE
+#define GetLocalTime GetLocalTimeWINCE
+#endif


I think the proper thing to do is simply rename GetLocalTime in this file to something else.  I am not sure what the name should be, suggestions? (js_GetLocalTime, myGetLocalTime?)
Not js_...  we don't call static functions by that name.

How about GetAndCacheLocalTime for ours?
Attached patch rename GetLocalTime. (obsolete) — Splinter Review
Attachment #304763 - Flags: review?(crowder)
Comment on attachment 304763 [details] [diff] [review]
rename GetLocalTime.

+    if (!GetAndCacheLocalTime(cx, obj, NULL, &localtime) || JSDOUBLE_IS_NaN(localtime))
This line becomes longer than 80-characters.  Needs to be split at the ||, see nearby code for a style sample.

It's unfortunate that our naming style for statics clashes with the way Windows API functions are named.  :(
Attachment #304763 - Flags: review?(crowder) → review-
Attached patch patch v.2 (obsolete) — Splinter Review
Attachment #304763 - Attachment is obsolete: true
Attachment #304783 - Flags: review?(crowder)
Comment on attachment 304783 [details] [diff] [review]
patch v.2

Oh, I should've warned you.  :(  When splitting a conditional across lines, Spidermonkey style demands braces around even one-line "then" clauses.  One more spin, please?  Sorry.
Attached patch patch v.3Splinter Review
with the right braces.
Attachment #304783 - Attachment is obsolete: true
Attachment #304783 - Flags: review?(crowder)
Comment on attachment 304801 [details] [diff] [review]
patch v.3

Looks good, thanks!
Attachment #304801 - Flags: review+
Attachment #304801 - Flags: approval1.9?
Comment on attachment 304801 [details] [diff] [review]
patch v.3

a=beltzner for 1.9
Attachment #304801 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: checkin-needed
Comment on attachment 304801 [details] [diff] [review]
patch v.3

mozilla/js/src/jsdate.c 	3.102
Keywords: checkin-needed
Whiteboard: checkin-needed
fixed.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
and verified.
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.