Closed Bug 418665 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
All
Windows Mobile 6 Standard
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: dougt, Assigned: dougt)

Details

Attachments

(1 file, 2 obsolete files)

patch v.3
5.10 KB, patch
crowderbt
: review+
beltzner
: approval1.9+
Details | Diff | Splinter Review 
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.3
        — 
          — Splinter 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: 15 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.