Re: [ros-dev] [ros-diffs] [cfinck] 28423: - Always include"wine/unicode.h" before all other headers, when we need the wchar_t type. unicode.h includes windef.h, which includes winnt.h, which has the handling for the wchar_t type. As it's the firstinc
On 8/19/07, Colin Finck mail@colinfinck.de wrote:
These changes were made to properly compile ReactOS under a Mac OS X host. They have nothing to do with MSVC or the Microsoft SDK headers.
Sorry I was not clear about what I meant in my rant and sounded bitchy
Mac OS X defines its own wchar_t type in "ctype.h". But it uses _WCHAR_T for reporting that the type has been defined and not _WCHAR_T_DEFINED like Windows does. This is why I needed to add a handling for _WCHAR_T in "winnt.h". But if "ctype.h" is now included before "winnt.h", these changes don't have any effect. This is why I changed the header order.
Of course, there might be other "solutions" to this problem (like adding all possible definitions that wchar_t has been defined into the Makefile), but this is the simplest one. And I see no reason not to do this for fixing this problem.
It makes anyone doing a diff have more stuff to have to workaround. There are constantly minor updates and fixes to the unicode tables in Wine because Microsoft did not just generate them from the uncode.org spec but added a bunch of stuff. Wine currently builds on OS X with the header order as it stands and figures out someway around it. I guess its using a define in the makefile for _WCHAR_T generated by configure or something. I have not looked so don't quote me on that. I don't really care if you want to change it to make it less of a hassle to build on OS X for ReactOS but its just going to break again next time someone syncs the unicode lib so its better if you can work around it in the makefile.
I agree with Steven on this, these changes should be avoided. Many other changes along with this one also have the same problem. (e.g. r28413)
-----Original Message----- From: ros-dev-bounces@reactos.org [mailto:ros-dev-bounces@reactos.org] On Behalf Of Steven Edwards Sent: 19 August 2007 21:41 To: ReactOS Development List Subject: Re: [ros-dev] [ros-diffs] [cfinck] 28423: - Alwaysinclude"wine/unicode.h" before all other headers,when we need the wchar_t type. unicode.h includes windef.h,which includes winnt.h,which has the handling for the wchar_t type. As it's the firstinc
On 8/19/07, Colin Finck mail@colinfinck.de wrote:
These changes were made to properly compile ReactOS under a Mac OS X host. They have nothing to do with MSVC or the Microsoft SDK headers.
Sorry I was not clear about what I meant in my rant and sounded bitchy
Mac OS X defines its own wchar_t type in "ctype.h". But it uses _WCHAR_T for reporting that the type has been defined and not _WCHAR_T_DEFINED like Windows does. This is why I needed to add a handling for _WCHAR_T in "winnt.h". But if "ctype.h" is now included before "winnt.h", these changes don't have any effect. This is why I changed the header order.
Of course, there might be other "solutions" to this problem (like adding all possible definitions that wchar_t has been defined into the Makefile), but this is the simplest one. And I see no reason not to do this for fixing this problem.
It makes anyone doing a diff have more stuff to have to workaround. There are constantly minor updates and fixes to the unicode tables in Wine because Microsoft did not just generate them from the uncode.org spec but added a bunch of stuff. Wine currently builds on OS X with the header order as it stands and figures out someway around it. I guess its using a define in the makefile for _WCHAR_T generated by configure or something. I have not looked so don't quote me on that. I don't really care if you want to change it to make it less of a hassle to build on OS X for ReactOS but its just going to break again next time someone syncs the unicode lib so its better if you can work around it in the makefile.
-- Steven Edwards
"There is one thing stronger than all the armies in the world, and that is an idea whose time has come." - Victor Hugo _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. If verification is required please request a hard-copy version.
Amteus Secure Communications Ltd 57 Cardigan Lane, Leeds, LS4 2LE t: +44 (0) 870 8368770 f: +44 (0) 870 8368701
Registered in England No 4760795
On 8/19/07, Steven Edwards wrote: It makes anyone doing a diff have more stuff to have to workaround.
Agreed, so we should come to a common solution with Wine.
I guess its using a define in the makefile for _WCHAR_T generated by configure or something.
Might be possible, but I don't see this as a good solution. As Ged already mentioned, I also changed this in wrc and other components.
If you look in glibc's "stddef.h" (for example here: http://tinyurl.com/yv6s9b), you find 32 checks for the existence of the wchar_t type. We'll probably never need all of them, but this just shows that almost every host needs its own check. And adding all these definitions to the Makefile of every app, which needs them, is a huge overkill in my opinion. This is why I prefer putting all checks into "winnt.h".
I don't want to sound harsh here, but couldn't maybe Wine adapt our solution as well?
Regards,
Colin
On 8/20/07, Colin Finck mail@colinfinck.de wrote:
If you look in glibc's "stddef.h" (for example here: http://tinyurl.com/yv6s9b), you find 32 checks for the existence of the wchar_t type. We'll probably never need all of them, but this just shows that almost every host needs its own check. And adding all these definitions to the Makefile of every app, which needs them, is a huge overkill in my opinion. This is why I prefer putting all checks into "winnt.h".
I agree its a pain but it seems like a GNUism we have to live with. I mean any other project we import in to the tree is going to have the same problem if it uses the wchar_t type right? So we would have to change the include order with them as well. Couldn't we add some magic to rbuild to make it act like configure, detect the host and then pass this to the makefile by just globally add a -D($HOST_WCHAR_DEFINE) like we do with other compiler flags?
Sorry if I am way off base, I admit I have not looked in to the problem in depth, I am just trying to avoid long term hassle. As it stands now the Wine guys are reluctant to change things just for ReactOS and also reluctant to change things that already work on multiple host platforms.
wine / uncode header can be remove any time. and it shall only be use for wine part, this header take over the orginal headers of unicode.h it is not a accpect solvtions. as steven say and others.
----- Original Message ----- From: "Steven Edwards" winehacker@gmail.com To: "ReactOS Development List" ros-dev@reactos.org Sent: Monday, August 20, 2007 8:02 PM Subject: Re: [ros-dev] [ros-diffs] [cfinck] 28423: -Alwaysinclude"wine/unicode.h" before all other headers,when we need the wchar_t type. unicode.h includes windef.h,which includes winnt.h,which has the handling for the wchar_t type. As it's the firstinc
On 8/20/07, Colin Finck mail@colinfinck.de wrote:
If you look in glibc's "stddef.h" (for example here: http://tinyurl.com/yv6s9b), you find 32 checks for the existence of the wchar_t type. We'll probably never need all of them, but this just shows that almost
every
host needs its own check. And adding all these definitions to the Makefile of every app, which
needs
them, is a huge overkill in my opinion. This is why I prefer putting all checks into "winnt.h".
I agree its a pain but it seems like a GNUism we have to live with. I mean any other project we import in to the tree is going to have the same problem if it uses the wchar_t type right? So we would have to change the include order with them as well. Couldn't we add some magic to rbuild to make it act like configure, detect the host and then pass this to the makefile by just globally add a -D($HOST_WCHAR_DEFINE) like we do with other compiler flags?
Sorry if I am way off base, I admit I have not looked in to the problem in depth, I am just trying to avoid long term hassle. As it stands now the Wine guys are reluctant to change things just for ReactOS and also reluctant to change things that already work on multiple host platforms.
-- Steven Edwards
"There is one thing stronger than all the armies in the world, and that is an idea whose time has come." - Victor Hugo _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Magnus Olsen wrote: wine / uncode header can be remove any time.
I moved Wine's "unicode.h" header up, because it includes "windef.h".
The fix doesn't depend on Wine's "unicode.h". I could have also just added an "#include <windef.h>" line before "ctype.h" is included. I moved the inclusion of "unicode.h" up, because it was the simpler solution in my opinion.
Regards,
Colin
Colin Finck ha scritto:
And adding all these definitions to the Makefile of every app, which needs them, is a huge overkill in my opinion. This is why I prefer putting all checks into "winnt.h".
What I want to ask is: why are we including host headers from target headers?
KJK::Hyperion wrote: What I want to ask is: why are we including host headers from target headers?
I assume, you're referring to the "ctype.h" included by "winnt.h". I think, we include the host header just because we don't have the Standard C Library headers in our source tree. For these headers, we have to depend on the host headers.
Under Windows, this is no problem as RosBE contains these headers, so they are quite consistent. But under other hosts, these headers are often slightly different.
Regards,
Colin
Colin Finck ha scritto:
What I want to ask is: why are we including host headers from target headers?
I assume, you're referring to the "ctype.h" included by "winnt.h". I think, we include the host header just because we don't have the Standard C Library headers in our source tree.
That is horrible and you should wash your mouth with soap. Just how the hell can we have the host include path by default in the target compiler? that sounds stupid. And, impossible. It sounds like, in fact, we are using target headers in host code, and, by extension, mixing target headers in host tool headers. Which is which, Colin?
For these headers, we have to depend on the host headers.
No, resent and repent. MinGW must have all the target headers we need. If it doesn't, we provide them. It's not so hard. Host is host and target is target. You don't include source headers in target sources
But under other hosts, these headers are often slightly different.
The solution, my good man, is to port RosBE to other hosts
KJK::Hyperion ha scritto:
You don't include source headers in target sources
Read: host headers in target sources
KJK::Hyperion wrote: Just how the hell can we have the host include path by default in the target compiler?
I didn't say that the target compiler uses the host include path. I just talked about the host headers. But maybe this was a bit confusing: When you compile a GCC cross-compiler and specify the --with-headers parameter, it will copy some important missing headers from the host include path into the target include path and run "fixincludes" to make them compatible with GCC.
For Linux and FreeBSD, this includes the "stddef.h" header, which is also responsible for defining wchar_t on these hosts. Don't know about Mac OS X as Alex did the tests on that host.
It sounds like, in fact, we are using target headers in host code
True. Since we share the code for some libraries for host and target components, we consequently mix target headers and host code.
The solution, my good man, is to port RosBE to other hosts
Right, this will most-probably reduce the problems compiling ReactOS on different hosts. But if GCC continues using some headers, which are derived from the host headers, we will still need different fixes for different hosts.
Regards,
Colin
Colin Finck ha scritto:
For Linux and FreeBSD, this includes the "stddef.h" header, which is also responsible for defining wchar_t on these hosts.
So this is a MinGW bug, where not hosts are created equal. Isn't it? wchar_t must not depend on the host. wchar_t is not the happy-fun ISO C generic type, nice and kind and asexual. wchar_t under MinGW must be the hairy Win32 bubba, two bytes unsigned. The happy-fun ISO C wchar_t will get beaten to the ground and mugged. It has no business in my <stddef.h>
It sounds like, in fact, we are using target headers in host code
True.
ò___ò
WHY and WHERE
Since we share the code for some libraries for host and target components, we consequently mix target headers and host code.
"Consequently"? I hope that's "accidentally". When host tools and libraries are compiled under Windows, they must be compiled as Windows code. When they are compiled under Linux, as Linux code. And so on. No exceptions. NO. EXCEPTIONS. We have been having these idiotic issues for YEARS, but the difference between host and target is pretty simple. Host is host and target is target. You will never fix
What host modules include target headers?
KJK::Hyperion wrote: So this is a MinGW bug, where not hosts are created equal. Isn't it?
Kind of. The mingw-runtime contains some files like "stdio.h", which include "stddef.h". But mingw-runtime does not contain any "stddef.h" file, so GCC's fixincludes tool copies it from the host.
wchar_t must not depend on the host.
Only for the target compiler or also for the host compiler?
As long as the "stddef.h" is not consistent for the target compiler on different hosts, we at least need to report back that wchar_t has been defined. And the inclusion order needs to be correct, so our wchar_t gets defined before it can be defined by "stddef.h". But if "stddef.h" would be consistent for the target compiler on all hosts, this problem would be gone.
Concerning the host compiler: On the one side, you wrote that tools under Windows have to be compiled as Windows code, under Linux as Linux code, etc. So in some cases, a tool might require a system wchar_t. And the size of this wchar_t depends on the host. On the other side, we always use 2-byte unsigned wchar_t's under Windows, so our host tools probably only need this type. But this can depend on the situation...
What host modules include target headers?
Don't understand me wrong here. The standard C headers (like "stdio.h", "stdlib.h", etc.) are correctly separated between host and target (or at least, should be ;-).
What we share between both are some of the Win32 headers like "windef.h" or "winnt.h". An example of this is cmlib: It heavily uses Win32 types like ULONG and UCHAR, so it has to include "winnt.h" for defining them. cmlib is shared between host and target components, like mkhive (host) and freeldr (target).
As these headers only define other names for generic types, I see no reason for not using them in host code as well.
Regards,
Colin
Colin Finck ha scritto:
wchar_t must not depend on the host.
Only for the target compiler or also for the host compiler?
Why, for the target of course
So in some cases, a tool might require a system wchar_t. And the size of this wchar_t depends on the host.
THEN DON'T USE WCHAR_T. The nice asexual happy-fun wchar_t makes no guarantee. It lives in a simpler world. It's not meant as a portable format. Look at Samba, for fuck's sake. Go tell them to use wchar_t for the Unicode strings in the protocol
On the other side, we always use 2-byte unsigned wchar_t's under
Windows, so
our host tools probably only need this type. But this can depend on the situation...
No. This is defending programmers that cry "mommy" and want to use wchar_t because it's what all the cool kids use. We are not the cool kids. If an on-disk format says "Unicode", it's little-endian UTF-16, that only ACCIDENTALLY and OCCASIONALLY happens to be the same as wchar_t, and we uncool kids suck it up and use an array of USHORTs (uint16_t to be nitpicky) instead. If you don't get this, you have the touch of death and every single of your contributions drives another nail in this project's coffin. I'm dead serious. Only an open source project could afford this kind of shit
What we share between both are some of the Win32 headers like
"windef.h" or
"winnt.h".
NOWAI!!! Color me wholly unsurprised
An example of this is cmlib: It heavily uses Win32 types like ULONG and UCHAR, so it has to include "winnt.h" for defining them.
Do you think Microsoft uses winnt.h for such purposes? no way. winnt.h is a mosaic of header file fragments, and internal sources include a minimal set of appropriate header fragments. When this project will stop sucking the fake tit of "using the official Microsoft headers", maybe this kind of issue will go away
As these headers only define other names for generic types, I see no
reason
for not using them in host code as well.
"Only define other names for generic types"? of course that's not half of what they do. Stop fooling yourself. I mean THINK ABOUT IT FOR ONE SECOND. If you were to write a Linux program, would you include winnt.h for some typedefs, or find an alternative solution first? For example writing code meant to be portable in a portable manner, Alex Ionescu be damned, or creating more minimal header files for standard definitions
Colin Finck wrote:
I guess its using a define in the makefile for _WCHAR_T generated by configure or something.
Might be possible, but I don't see this as a good solution.
It's a better solution than what you changed. I haven't looked at it in any depth, but that change looks wrong. "wrc.h" should be at the base of the headers and any requireent for standard headers above it.
I don't want to sound harsh here, but couldn't maybe Wine adapt our
solution
as well?
We can't expect Wine to accept what appears to be incorrect changes to their working code. It works for them, we need to make it work for us.
I've been chatting to Hervé about this and here are at least 3 potential ways to fix this:
- change our wchar_t check to detect Mac OS X hosts - add a ros-specific ctype.h - change the makefile to add the required stuff : ifeq (${PLATFORM}, MACOSX) *mac stuff*
Ged.
This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secure or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. If verification is required please request a hard-copy version.
Amteus Secure Communications Ltd 57 Cardigan Lane, Leeds, LS4 2LE t: +44 (0) 870 8368770 f: +44 (0) 870 8368701
Registered in England No 4760795
Ged wrote:
Colin Finck wrote:
I guess its using a define in the makefile for _WCHAR_T
generated by
configure or something.
Might be possible, but I don't see this as a good solution.
It's a better solution than what you changed.
But it's not very intelligent, when we need to add more definitions. This would require changing all Makefiles, which do it this way. By putting the check into "winnt.h", we have a centralized place, where this would need to be changed.
By the way, what are the rules for this Wine imported code? Can we only change the Makefile or also make minor changes to the code? Cause I think, another solution might be adding a ReactOS-specific header at the top of each source file, which then takes care of this wchar_t thing.
I've been chatting to Hervé about this and here are at least 3 potential ways to fix this:
- change our wchar_t check to detect Mac OS X hosts
What do you mean by "our wchar_t check"? In my opinion, our wchar_t check is in "winnt.h" and there I changed it for Mac OS X.
- add a ros-specific ctype.h
This probably leads to problems as every host might need an own "ctype.h". Remember that "ctype.h" also defines other types, not just wchar_t. And some of our host applications probably need these types defined correctly, so they work on this particular host.
Of course, we could add lots of host checks to our "ctype.h", but in my opinion, this is a huge overkill and difficult to maintain.
- change the makefile to add the required stuff : ifeq (${PLATFORM}, MACOSX) *mac stuff*
Not a good solution if this needs to be changed in every Makefile...
Regards,
Colin
-
Colin Finck -
Ged -
KJK::Hyperion -
KJK::Hyperion -
Magnus Olsen -
Steven Edwards