Hello, We are looking at porting more code from WINE to ReactOS and I am having trouble understanding the need for libwine_unicode as a shared library. Some of the functions that it exports such as CompareString are implemented in ntdll as RtlCompareString and then also used in shlwapi as per a recent patch. Wouldnt it better to import the function from ntdll and hide libwine_unicode or make the parts that we need to share a static library? In ReactOS we have created NT compatible NLS files. Could WINE do this also and dump libwine_nicode?
Also I was looking at the HeapAlloc stuff in WINE and code we have ported from ReactOS such as the FormatMessage function in kernel32. Doesnt kernel32 HeapAlloc and friends map on top of ntdll RtlHelpAlloc? Can we submit patches to WINE so that everything just calls RtlHeapAlloc so we dont have to change the sources?
Thanks Steven
__________________________________ Do you Yahoo!? Free Pop-Up Blocker - Get it now http://companion.yahoo.com/
Steven Edwards wrote:
Hello, We are looking at porting more code from WINE to ReactOS and I am having trouble understanding the need for libwine_unicode as a shared library. Some of the functions that it exports such as CompareString are implemented in ntdll as RtlCompareString and then also used in shlwapi as per a recent patch. Wouldnt it better to import the function from ntdll and hide libwine_unicode or make the parts that we need to share a static library? In ReactOS we have created NT compatible NLS files. Could WINE do this also and dump libwine_nicode?
We won't drop libwine_unicode (because we use it other pure unix programs, so using only ntdll isn't a solution). However, as the process of separating ntdll and kernel32 isn't over yet, NLS functions haven't looked on so far. So, removing wine_unicode from kernel32 and using only ntdll's exports could be a target. But, this isn't obvious and may require some tweaking (especially at process startup time).
Also I was looking at the HeapAlloc stuff in WINE and code we have ported from ReactOS such as the FormatMessage function in kernel32. Doesnt kernel32 HeapAlloc and friends map on top of ntdll RtlHelpAlloc? Can we submit patches to WINE so that everything just calls RtlHeapAlloc so we dont have to change the sources?
That would be doable yes (at least in kernel32). In other DLLs, it's more arguable, especially if we want to test them on non NT systems.
A+
On December 1, 2003 02:44 pm, Eric Pouech wrote:
That would be doable yes (at least in kernel32). In other DLLs, it's more arguable, especially if we want to test them on non NT systems.
Agreed, I see no need to make our DLLs depend on ntdll. And I don't understand why you guys need to change the source for such a trivial (and useless) 'optimization'. Even within kernel32 I'd still use the Heap*() functions for consistency, no need to confuse people...
--- "Dimitrie O. Paun" dpaun@rogers.com wrote:
Agreed, I see no need to make our DLLs depend on ntdll. And I don't understand why you guys need to change the source for such a trivial (and useless) 'optimization'. Even within kernel32 I'd still use the Heap*() functions for consistency, no need to confuse people...
Ahhh forget the HeapAlloc stuff. That was my fault. Someone decided in a very drain-bamaged way to forward all the Heap* calls to the RtlHeap calls in the spec file rather than in the source so anytime we try to port Winehq sources for kernel32 that use HeapAlloc I was running in to problems using it internal.
Thanks Steven
__________________________________ Do you Yahoo!? Free Pop-Up Blocker - Get it now http://companion.yahoo.com/
Ahhh forget the HeapAlloc stuff. That was my fault. Someone decided in a very drain-bamaged way to forward all the Heap* calls to the RtlHeap calls in the spec file rather than in the source so anytime we try to port Winehq sources for kernel32 that use HeapAlloc I was running in to problems using it internal.
No, thats not brain damaged. Thats the way Microsoft does things. (at least on XP) HeapAlloc doesnt exist at all, all it is is a forwarder to NTDLL. Therefore, all the code in kernel32 should just call RtlAllocateHeap. Given that kernel32.dll is very much linked to ReactOS and wont run anywhere else there should be no problem.
Any other code that needs HeapAlloc can just call it and will pull it in from kernel32.dll.
Problem solved :) If there is an issue with code from kernel32.dll of wine that uses HeapAlloc, the solutions are: 1.implement ntdll.dll on wine and add RtlAllocateHeap to it. Then modify WINE kernel32.dll to call RtlAllocateHeap and also to forward HeapAlloc to RtlAllocateHeap. Since kernel32.dll of wine is not going to need to run on systems without RtlAllocateHeap, this shouldnt be an issue. But, if there is some objection to this, you could: 2.add a macro to a rectos-only header that gets yanked into the WINE ported kernel code like this. #define HeapAlloc RtlAllocateHeap That way, WINE can keep their HeapAlloc calls inside kernel32 (which one would assume they need for some reason otherwise point 1 above would be a valid solution) but things will compile on ReactOS without a need to make changes. Still, going the right way (i.e. adding RtlAllocateHeap to wine) is preferable.
--- Jonathan Wilson jonwil@tpgi.com.au wrote:
No, thats not brain damaged. Thats the way Microsoft does things. (at least on XP) HeapAlloc doesnt exist at all, all it is is a forwarder to NTDLL. Therefore, all the code in kernel32 should just call RtlAllocateHeap. Given that kernel32.dll is very much linked to ReactOS and wont run anywhere else there should be no problem.
Its already implemented in Wine ntdll. It was my mistake. BTW: Do they forward it by linker magic like we did on ReactOS or did they do it inline like in Wine?
http://cvs.winehq.com/cvsweb/wine/dlls/kernel/heap.c?rev=1.4&content-typ...
/* These are needed so that we can call the functions from inside kernel itself */
LPVOID WINAPI HeapAlloc( HANDLE heap, DWORD flags, SIZE_T size ) { return RtlAllocateHeap( heap, flags, size ); }
BOOL WINAPI HeapFree( HANDLE heap, DWORD flags, LPVOID ptr ) { return RtlFreeHeap( heap, flags, ptr ); }
LPVOID WINAPI HeapReAlloc( HANDLE heap, DWORD flags, LPVOID ptr, SIZE_T size ) { return RtlReAllocateHeap( heap, flags, ptr, size ); }
SIZE_T WINAPI HeapSize( HANDLE heap, DWORD flags, LPVOID ptr ) { return RtlSizeHeap( heap, flags, ptr ); }
I am just going to add this code to ReactOS in kernel32 and then its "problem solved". Wine can still attempt to run what ever dll/program under Win9x for testing that calls HeapAlloc and we dont have to make changes to all of the Wine sources.
Thanks Steven
__________________________________ Do you Yahoo!? Free Pop-Up Blocker - Get it now http://companion.yahoo.com/
ReactOS uses linker magic. My point is that there is no valid reason for the WINE code in kernel32 not to use RtlAllocateHeap since the WINE kernel32 code is only intended to run on ReactOS (where RtlAllocateHeap is available) and WINE (where RtlAllocateHeap is available). Therefore, re-writing wine kernel32 to call RtlAllocateHeap instead of HeapAlloc and perhaps changing WINE to do the linker forwarding foo (if such a thing is possible) is a good solution.
Other parts of the code in both ReactOS and WINE (dlls, tests or whatever else may be run on windows 9x) can just pull in kernel32.HeapAlloc like any other windows app would.
As for libwine_unicode, I havent looked but I suspect that on real windows, most of what libwine_unicode does is inside ntdll.dll somewhere. Therefore, lets put the libwine_unicode code into wine ntdll.dll, add the forwarding hooks needed from kernel32.dll that call ntdll.dll and then make WINE apps call kernel32.dll instead of libwine_unicode. Problem solved, plus its how MS does things AFAIK (which is always IMO the way that WINE and/or ReactOS should do things)
Jonathan Wilson jonwil@tpgi.com.au writes:
ReactOS uses linker magic. My point is that there is no valid reason for the WINE code in kernel32 not to use RtlAllocateHeap since the WINE kernel32 code is only intended to run on ReactOS (where RtlAllocateHeap is available) and WINE (where RtlAllocateHeap is available). Therefore, re-writing wine kernel32 to call RtlAllocateHeap instead of HeapAlloc and perhaps changing WINE to do the linker forwarding foo (if such a thing is possible) is a good solution.
There's no reason whatsoever to do that. HeapAlloc is a kernel function and it's perfectly OK to use it in kernel. It requires a bit of magic to link properly but that's not a problem.
As for libwine_unicode, I havent looked but I suspect that on real windows, most of what libwine_unicode does is inside ntdll.dll somewhere. Therefore, lets put the libwine_unicode code into wine ntdll.dll, add the forwarding hooks needed from kernel32.dll that call ntdll.dll and then make WINE apps call kernel32.dll instead of libwine_unicode.
It won't work, libwine_unicode is used in places that don't have access to ntdll (like the tools or the wine server).
--- "Dimitrie O. Paun" dpaun@rogers.com wrote:
On December 1, 2003 02:44 pm, Eric Pouech wrote:
That would be doable yes (at least in kernel32). In other DLLs,
it's
more arguable, especially if we want to test them on non NT
systems.
Agreed, I see no need to make our DLLs depend on ntdll. And I don't understand why you guys need to change the source for such a trivial (and useless) 'optimization'. Even within kernel32 I'd still use the Heap*() functions for consistency, no need to confuse people...
Sorry for making the noise about this. I am just trying to make porting code from WINE to ReactOS as idiot proof as possible. I really dont want to confuse people. The Heap* stuff I have figured out and that was a ReactOS problem that I am going to fix tongiht.
We have fixed the dependancy on libwine by implementing the WINE debug macros TRACE, ERR, etc on top of ReactOS DPRINT and DPRINT1 and then by making the wine debugchannel support a small static lib. Its still going to be called libwine to keep things as ID10T proff as possible. All FIXME messages are implemented as DPRINT1 so we will always see them. If someone needs more debug information then we can rebuild the WINE dlls under ReactOS with DBG = 1 in the reactos/config then you will get TRACE, ERR and WARN messages for that module also.
Now there is the issue of libwine_unicode.dll. I really dont think we need this in ReactOS the same way WINE does. We already have a few static libs for some of the Rtl* and string function so maybe I just need to implement this there. The recent CompareString changes in shlwapi and lbwine_uni got me thinking about it as ntdll also implements RtlCompareString.
Anyway I am just looking for suggestions on how to make porting/sharing this code simpler. We have most of the ground work laid and its mainly just cleanup now. Once these changes are implemented, more ReactOS developers will be able to start working on the port.
I am working on setting up one of my servers with a cross-compiler and the needed hacksed sources so I can still make bledding edge snapshots of Winehq CVS avaliable to anyone that wants to play with the latest Wine on Windows or ReactOS.
Thanks Steven
__________________________________ Do you Yahoo!? Free Pop-Up Blocker - Get it now http://companion.yahoo.com/
Steven Edwards steven_ed4153@yahoo.com writes:
Also I was looking at the HeapAlloc stuff in WINE and code we have ported from ReactOS such as the FormatMessage function in kernel32. Doesnt kernel32 HeapAlloc and friends map on top of ntdll RtlHelpAlloc? Can we submit patches to WINE so that everything just calls RtlHeapAlloc so we dont have to change the sources?
Why do you need that? Don't you have HeapAlloc etc. on ReactOS?
-----Original Message----- From: wine-devel-admin@winehq.org [mailto:wine-devel-admin@winehq.org]On Behalf Of Steven Edwards Sent: 01 December 2003 00:03 To: wine-devel@winehq.com Subject: Question about libwine_unicode functions and others in WINE
Hello, We are looking at porting more code from WINE to ReactOS and I am having trouble understanding the need for libwine_unicode as a shared library. Some of the functions that it exports such as CompareString are implemented in ntdll as RtlCompareString and then also used in shlwapi as per a recent patch. Wouldnt it better to import the function from ntdll and hide libwine_unicode or make the parts that we need to share a static library? In ReactOS we have created NT compatible NLS files. Could WINE do this also and dump libwine_nicode?
As noted by others, we don't want to create an unnecessary dependency on ntdll and more importantly we don't want to scare people off by using "undocumented" functions everywhere. However, couldn't we just replace the libwine_unicode function with an appropriate (well-documented) kernel32 string function: strcatW -> lstrcatW strcmpW -> lstrcmpW strcpyW -> lstrcpyW strlenW -> lstrlenW ... etc It may add a little bit of overhead in Wine (which we could possibly solve by adding #define's for e.g. lstrcatW -> strcatW), but it would solve the problem. We will still use libwine_unicode for the backend implementation of the string functions and for the tools, but it wouldn't matter for ReactOS. It would probably be best to add this as another janitorial project if we go for it. You can quite easily see which dlls use libwine_unicode as it links to it in the makefile.
Rob
"Robert Shearman" R.J.Shearman@warwick.ac.uk wrote:
However, couldn't we just replace the libwine_unicode function with an appropriate (well-documented) kernel32 string function: strcatW -> lstrcatW strcmpW -> lstrcmpW strcpyW -> lstrcpyW strlenW -> lstrlenW ... etc
No. The above APIs use SEH internally which make things a lot slower. I think ReactOS wants to avoid them as well, as much as possible and use some internal versions instead.
"Robert Shearman" R.J.Shearman@warwick.ac.uk writes:
However, couldn't we just replace the libwine_unicode function with an appropriate (well-documented) kernel32 string function: strcatW -> lstrcatW strcmpW -> lstrcmpW strcpyW -> lstrcpyW strlenW -> lstrlenW ... etc It may add a little bit of overhead in Wine (which we could possibly solve by adding #define's for e.g. lstrcatW -> strcatW), but it would solve the problem.
Yes, this is something I've been wanting to do for a long time. Not so much because of libwine_unicode, but because a lot of places use the lstr* functions for no reason, and this slows things down quite a bit. The problem is that some places do rely on the exception handling that the lstr* functions do, so we'd first need to go through every call to check whether we need to add an exception handler in the caller.
-----Original Message----- From: Alexandre Julliard [mailto:julliard@winehq.com] Sent: 02 December 2003 19:26 To: Robert Shearman Cc: Steven Edwards; wine-devel@winehq.org Subject: Re: Question about libwine_unicode functions and others in WINE
"Robert Shearman" R.J.Shearman@warwick.ac.uk writes:
However, couldn't we just replace the libwine_unicode function with an appropriate (well-documented) kernel32 string function: It may add a little bit of overhead in Wine (which we could
possibly solve
by adding #define's for e.g. lstrcatW -> strcatW), but it would
solve the
problem.
Yes, this is something I've been wanting to do for a long time. Not so much because of libwine_unicode, but because a lot of places use the lstr* functions for no reason, and this slows things down quite a bit. The problem is that some places do rely on the exception handling that the lstr* functions do, so we'd first need to go through every call to check whether we need to add an exception handler in the caller.
Just another thought: why don't we use the wcscat, wcslen, wcscpy functions from ntdll? AFAIK, they are completely compatible with the msvcrt ones with the same names. The lstr* calls that don't need exception handling can link to these instead. On other platforms ReactOS, Windows, etc they can link with either msvcrt or ntdll, whichever is more convenient. Is this what Steven originally suggested?
Rob
--- Robert Shearman R.J.Shearman@warwick.ac.uk wrote:
Just another thought: why don't we use the wcscat, wcslen, wcscpy functions from ntdll? AFAIK, they are completely compatible with the msvcrt ones with the same names. The lstr* calls that don't need exception handling can link to these instead. On other platforms ReactOS, Windows, etc they can link with either msvcrt or ntdll, whichever is more convenient. Is this what Steven originally suggested?
Yes this is what I was asking about originally and yes we should use them internaly but the problem is wine/tools programs cant link to ntdll or msvcrt wc* functions because they are needed to build wine. A header file that converts libwine_unicode functions to the native wc* functions should be enough for the most part. Sorta like the RtlHeapAlloc->HeapAlloc wrapping that we see in kernel32.
Thanks Steven
__________________________________ Do you Yahoo!? Free Pop-Up Blocker - Get it now http://companion.yahoo.com/
"Robert Shearman" R.J.Shearman@warwick.ac.uk writes:
Just another thought: why don't we use the wcscat, wcslen, wcscpy functions from ntdll? AFAIK, they are completely compatible with the msvcrt ones with the same names. The lstr* calls that don't need exception handling can link to these instead. On other platforms ReactOS, Windows, etc they can link with either msvcrt or ntdll, whichever is more convenient. Is this what Steven originally suggested?
The problem is that the wcs* function deal with wchar_t, and this is 32-bit on Unix. We would have to use WCHAR instead, but it would get pretty confusing for people who know the wcs* functions, so it's IMO better to use Windows APIs that don't have a Unix equivalent.
On December 2, 2003 02:25 pm, Alexandre Julliard wrote:
Yes, this is something I've been wanting to do for a long time. Not so much because of libwine_unicode, but because a lot of places use the lstr* functions for no reason, and this slows things down quite a bit. The problem is that some places do rely on the exception handling that the lstr* functions do, so we'd first need to go through every call to check whether we need to add an exception handler in the caller.
Should I add this as a janitorial task?
"Dimitrie O. Paun" dpaun@rogers.com writes:
Should I add this as a janitorial task?
I guess so, though I'm not sure how we should proceed. Maybe the first step would be to get rid of the lstr* calls, and then to add them back in a second step once every place has been checked.
On December 4, 2003 04:36 pm, Alexandre Julliard wrote:
I guess so, though I'm not sure how we should proceed. Maybe the first step would be to get rid of the lstr* calls, and then to add them back in a second step once every place has been checked.
Maybe we should come up with a bit more obvious names that are (1) easier to see are temporary and (2) easier to grep. Maybe a wl_/wstr_/wine_ prefix of sorts?
"Dimitrie O. Paun" dpaun@rogers.com writes:
Maybe we should come up with a bit more obvious names that are (1) easier to see are temporary and (2) easier to grep. Maybe a wl_/wstr_/wine_ prefix of sorts?
But then you'd need to rename all the existing uses of wine/unicode.h functions too. I don't think it's worth it. The current names are just fine, and they aren't necessarily temporary, we only need to replace them with lstr* where we want to get rid of libwine_unicode, which is clearly not possible everywhere.
-
Alexandre Julliard -
Dimitrie O. Paun -
Dmitry Timoshkov -
Eric Pouech -
Jonathan Wilson -
Robert Shearman -
Steven Edwards