474366 - OS/2 build break due to bug470914

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Walter Meinl]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.2a1pre) Gecko/20090120 Minefield/3.2a1pre
Build Identifier: 

E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp: In function 'JSBool GetLocationProperty(JSContext*, JSObject*, jsval, jsval*)':
E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp:130: error: expected ';' before 'JSStackFrame'
E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp:168: error: 'filenameString' was not declared in this scope
E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp:176: error: 'filenameString' was not declared in this scope
E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp: In function 'JSBool GC(JSContext*, JSObject*, uintN, jsval*, jsval*)':
E:/usr/src/hg/mozilla-central/js/src/xpconnect/shell/xpcshell.cpp:506: warning: format '%08lx' expects type 'long unsigned int', but argument 5 has type 'int'
make.exe[5]: *** [xpcshell.o] Error 1


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Walter Meinl]

Attachment: just a workaround to build (go with Unix)

Just an unbreaker for the moment

Comment 2

[(not currently active) Ted Mielczarek]

Oops, sorry. It was my intention that the code should build on all platforms, and just return early out of all those methods.

Comment 3

[Walter Meinl]

Attachment: really exclude unsupported platforms

(In reply to comment #2)
> Oops, sorry. It was my intention that the code should build on all platforms,
> and just return early out of all those methods.
Ted, I think this would really keep out unsupported platforms

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 358288 [details] [diff] [review]
really exclude unsupported platforms

Can you update this to take attachment 358017 [details] [diff] [review] into account? (Follow-up patch on bug 470914 to fix WinCE.)

Comment 5

[Walter Meinl]

Attachment: really exclude unsupported platforms including WINCE (checked in: comment 6)

(In reply to comment #4)
> (From update of attachment 358288 [details] [diff] [review])
> Can you update this to take attachment 358017 [details] [diff] [review] into account? (Follow-up patch on
> bug 470914 to fix WinCE.)
Oops, thought that the WinCE probs were already solved by this checkin

Comment 6

[Peter Weilbacher]

Pushed <http://hg.mozilla.org/mozilla-central/rev/1f7c2d158994>. This was only needed on trunk, right?

Comment 7

[Walter Meinl]

Attachment: doo, missed some brackets (checked in: comment 9)

(In reply to comment #4)
> (From update of attachment 358288 [details] [diff] [review])
> Can you update this to take attachment 358017 [details] [diff] [review] into account? (Follow-up patch on
> bug 470914 to fix WinCE.)

Ted, Peter, looking again at attachment 358017 [details] [diff] [review] I realized that I should have put some brackets around #if !not defined(XP_WIN) && !not defined(XP_UNIX) before || defined (WINCE) as it was done in that attachment.
the fix here takes in account that attachment 358476 [details] [diff] [review] was already pushed, thanks Peter.
And yes so far the fix is only needed for trunk

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 359796 [details] [diff] [review]
doo,  missed some brackets (checked in: comment 9)

I think order-of-operations binds it this way anyway, but doesn't hurt to make it explicit, certainly.

Comment 9

[Peter Weilbacher]

Yeah, doesn't really change anything, but pushed for clarity:
http://hg.mozilla.org/mozilla-central/rev/2cd22042f7ef

Comment 10

[Walter Meinl]

(In reply to comment #6)
> Pushed <http://hg.mozilla.org/mozilla-central/rev/1f7c2d158994>. This was only
> needed on trunk, right?

(In reply to comment #9)
> Yeah, doesn't really change anything, but pushed for clarity:
> http://hg.mozilla.org/mozilla-central/rev/2cd22042f7ef
Peter, these patches are needed now also for the 1.9.1 branch, see bug470914 comment #26.
in bug470914 #23 Ted says no approval is needed for xpcshell.

Comment 11

[Peter Weilbacher]

Done:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f3d4c7249d3a
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/af679d39641b