Bug 34378 - [Qt] WinCE buildfix after r52729
Summary: [Qt] WinCE buildfix after r52729
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-30 13:31 PST by Patrick R. Gansterer
Modified: 2010-02-01 16:39 PST (History)
2 users (show)

See Also:


Attachments
The patch (1.05 KB, patch)
2010-01-30 13:33 PST, Patrick R. Gansterer
ariya.hidayat: review+
Details | Formatted Diff | Diff
The patch (typo fixed) (1.12 KB, patch)
2010-02-01 02:44 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-01-30 13:31:23 PST
Rename WTF_PLATFORM_*_ENDIAN to WTF_CPU_*_ENDIAN
Comment 1 Patrick R. Gansterer 2010-01-30 13:33:45 PST
Created attachment 47774 [details]
The patch
Comment 2 Ariya Hidayat 2010-01-31 15:48:26 PST
Comment on attachment 47774 [details]
The patch

LGTM.

Maybe fix that typo (Q_BIG_EDIAN) as well?
Comment 3 Patrick R. Gansterer 2010-02-01 02:44:39 PST
Created attachment 47823 [details]
The patch (typo fixed)
Comment 4 Zoltan Herczeg 2010-02-01 04:19:30 PST
The patch looks good to me, although the comment says:

   We can not determine the endianess at compile time.

If this comment is not valid anymore, please remove or modify it. Furthermore, please do not set commit‑queue flags in the future. It should only be enabled after your patch is accepted.
Comment 5 Patrick R. Gansterer 2010-02-01 04:31:11 PST
(In reply to comment #4)
> The patch looks good to me, although the comment says:
> 
>    We can not determine the endianess at compile time.
> 
> If this comment is not valid anymore, please remove or modify it. Furthermore,
> please do not set commit‑queue flags in the future. It should only be enabled
> after your patch is accepted.
Currently i compile JSC without Qt. Platform.h "dedects" MIDDLE_ENDIAN which is wrong! So i think the comment is valid.

I want to change this to the following in a further patch:
#if OS(WINCE)
#   undef WTF_CPU_BIG_ENDIAN
#   undef WTF_CPU_MIDDLE_ENDIAN
#   if PLATFORM(QT)
#       include <QtGlobal>
#       if Q_BYTE_ORDER == Q_BIG_ENDIAN
#           define WTF_CPU_BIG_ENDIAN 1
#       endif
#   endif
#   include <ce_time.h>
#endif
Comment 6 Zoltan Herczeg 2010-02-01 06:50:32 PST
> I want to change this to the following in a further patch:

Fixing fixes is not really a WebKit style. Please do this change in patch.
Comment 7 Patrick R. Gansterer 2010-02-01 06:55:52 PST
(In reply to comment #6)
> > I want to change this to the following in a further patch:
> 
> Fixing fixes is not really a WebKit style. Please do this change in patch.

What should i do now?
Changing WTF_PLATFORM_*_ENDIAN to WTF_CPU_*_ENDIAN is _only_ a buildfix.
Should i change it to comment #5 or should i change the whole ARM ENDIAN DEDECTION (add ugly OS(WINCE))?
Comment 8 WebKit Commit Bot 2010-02-01 16:39:37 PST
Comment on attachment 47823 [details]
The patch (typo fixed)

Clearing flags on attachment: 47823

Committed r54176: <http://trac.webkit.org/changeset/54176>
Comment 9 WebKit Commit Bot 2010-02-01 16:39:42 PST
All reviewed patches have been landed.  Closing bug.