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

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
blocker
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
All
Windows Mobile 6 Standard
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

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

Comment 1

10 years ago
Not js_...  we don't call static functions by that name.

How about GetAndCacheLocalTime for ours?
(Assignee)

Comment 2

10 years ago
Created attachment 304763 [details] [diff] [review]
rename GetLocalTime.
Attachment #304763 - Flags: review?(crowder)

Comment 3

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

Comment 4

10 years ago
Created attachment 304783 [details] [diff] [review]
patch v.2
Attachment #304763 - Attachment is obsolete: true
Attachment #304783 - Flags: review?(crowder)

Comment 5

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

Comment 6

10 years ago
Created attachment 304801 [details] [diff] [review]
patch v.3

with the right braces.
Attachment #304783 - Attachment is obsolete: true
Attachment #304783 - Flags: review?(crowder)

Comment 7

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

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: checkin-needed

Comment 9

10 years ago
Comment on attachment 304801 [details] [diff] [review]
patch v.3

mozilla/js/src/jsdate.c 	3.102
Keywords: checkin-needed
Whiteboard: checkin-needed
(Assignee)

Comment 10

10 years ago
fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

10 years ago
and verified.
Status: RESOLVED → VERIFIED

Updated

10 years ago
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.