[Webkit-unassigned] [Bug 20746] Port WebKit to Qt on Windows CE

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 16 04:49:43 PDT 2008


https://bugs.webkit.org/show_bug.cgi?id=20746





------- Comment #23 from joerg.bornemann at trolltech.com  2008-09-16 04:49 PDT -------
(In reply to comment #18)

> Tab:
> +    time_t timet = mktime(&tmtime);

Will fix this.

> This feels strange, that generally the check is a compile check except for
> WIN_CE:
> +#if COMPILER(MSVC7) || COMPILER(MINGW) || PLATFORM(WIN_CE)
> 
> Makes me wonder which set of checks is wrong...

We've discussed this check too. Actually PLATFORM(WIN_CE) is both, a platform
check and a compiler check. For Windows CE MSVC uses a different compiler (and
different includes) than for Windows 32. We could add a MSVC_CE compiler define
which is always defined if PLATFORM(WINCE) and COMPILER(MSVC). But since
there's only one serious Windows CE compiler available, we didn't bother...

> It seems you do this in a couple places:
> 
> +#if PLATFORM(WIN_CE)
> +        /* On Windows CE there's no abort() and on regular Windows
> +           abort() exits with exit code 3. */
> +        exit(3);
> +#else
>          abort();
> +#endif
> 
> Maybe abort() should just be defined as an inline in some high-level header?

There are only two files where we do this:
JavaScriptCore/kjs/collector.cpp and 
JavaScriptCore/wtf/FastMalloc.cpp
Do we really want to infest the global namespace with an abort definition?

> Seems this should be an #else clause on the above HAVE(ERRNO_H):
> +#if PLATFORM(WIN_CE)
> +#define NO_ERRNO
> +#endif
> or?  Are there platforms which have errno but not in ERRNO_H?

Hm...you're right, it seems unlikely that there are platforms that define errno
but have no errno.h. Will fix this.

> Redundant, since you included it in Assertions.h as well:
> +
> +#if PLATFORM(WIN_CE)
> +#include <windows.h>
> +#endif
> +

Removed.

> Again, seems like the MSVC checks shoudl really be WIN_OS checks:
> +#if COMPILER(MSVC) && !PLATFORM(WIN_CE)
>  #ifndef WINVER

No, because WIN_OS is also defined for MinGW.

> Is there any cleaner way to do this?  In Chromium, we avoid including windows.h
> in any header.  I assume you'd probably want to do the same.  I think we avoid
> including it to avoid compile slowness or object file bloat.  But maybe I'm
> misinformed.

We had a little discussion on webkit-dev about this. The main problem is, that
WebKit defines the ASSERT macro. The Windows headers for Win CE define that too
and are broken in that sense that we *must* first include <windows.h> then
undefine ASSERT and define our own ASSERT macro.

> I'm somewhat confused by this statement:
> +/* PLATFORM(WIN_CE) && PLATFORM(QT)
> +   We can not determine the endianess at compile time. For
> +   Qt for Windows CE the endianess is specified in the
> +   device specific makespec
> +*/

For gcc on ARM you have several defines from which you can determine the
endianess of the platform. For MSVC for Windows CE this isn't possible because
the defines are missing. Qt has defines of the endianess in the mkspecs for the
specific platform.

> Why did you add:
> +#define HAVE_SYS_TYPES_H 1
> in 3 places in Platform.h?

To mark all the platforms that have sys/types.h which are DARWIN, Win32 and the
default case.

> I guess I'll r- this, requesting a second revision (possibly broken into
> smaller pieces).  I guess an alternative would be to find someone more
> windows-savvy who'd like to review the whole thing for you. :)

I hope that my comments explain a little bit. :)


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list