Ge van Geldorp gvg@reactos.com writes:
Send for review to wine-devel: http://www.winehq.org/hypermail/wine-devel/2004/11/0658.html (no comments) Submitted to wine-patch: http://www.winehq.org/hypermail/wine-patches/2004/11/0327.html Resubmitting against current CVS
Changelog: Ge van Geldorp gvg@reactos.com
- Generate and export stubs specified in .spec files for PE builds
I don't think we want to add a special mode just for stubs; they should really be replaced by proper functions (even if the functions are stubs themselves, at least they can print the parameters and try to return something meaningful instead of killing the app). You can view the lack of stubs support on PE as an encouragement to help us remove them from the spec files ;-)
From: julliard@winehq.org
I don't think we want to add a special mode just for stubs; they should really be replaced by proper functions (even if the functions are stubs themselves, at least they can print the parameters and try to return something meaningful instead of killing the app). You can view the lack of stubs support on PE as an encouragement to help us remove them from the spec files ;-)
Seems impossible for functions with unknown calling conventions and number of parameters.
Ge van Geldorp.
"Ge van Geldorp" gvg@reactos.com writes:
Seems impossible for functions with unknown calling conventions and number of parameters.
I don't think that's a common case, most of these have been identified by now. But anyway, if you have an app calling the function you should be able to determine the number of parameters; and if you don't then you shouldn't need the stub at all. Do you have a specific case where this is a problem?
From: Alexandre Julliard
Seems impossible for functions with unknown calling conventions and number of parameters.
I don't think that's a common case, most of these have been identified by now. But anyway, if you have an app calling the function you should be able to determine the number of parameters; and if you don't then you shouldn't need the stub at all. Do you have a specific case where this is a problem?
I was mixing native shell32.dll with Wine-derived shlwapi.dll. Native shell32.dll imports those stubs from shlwapi.dll but then doesn't call them in our usage scenarios. I'm just a little bit surprised that stubs are ok for elf builds but not for PE builds. It's not a big issue, I'm going to keep the code in ReactOS CVS anyway <g>.
Ge van Geldorp.
"Ge van Geldorp" gvg@reactos.com writes:
I was mixing native shell32.dll with Wine-derived shlwapi.dll. Native shell32.dll imports those stubs from shlwapi.dll but then doesn't call them in our usage scenarios. I'm just a little bit surprised that stubs are ok for elf builds but not for PE builds. It's not a big issue, I'm going to keep the code in ReactOS CVS anyway <g>.
Well, they really aren't OK for ELF builds either; the problem is we have added stubs as placeholders all over the place, while we should have only added them where there is a real need (mostly for ordinal imports). So now it will be a bit of work before we can get rid of them, but that should be the long term goal; and that's why I don't think we should add more code to support them.
On Tue, 14 Dec 2004 15:13:51 +0100, Alexandre Julliard wrote:
So now it will be a bit of work before we can get rid of them, but that should be the long term goal; and that's why I don't think we should add more code to support them.
I'm a bit confused, the specfile level stubs are handy if only because when an app calls them you can see exactly which function was called in the error. Otherwise you'd just get a crash at 0xdeadbeef (what if that address is mapped? can that even happen?).
It also suppresses all the warnings from the linker when using native DLLs that are linked against internal functions but never use them, for whatever reason.
They also make adding new DLLs easier, as you don't have to submit lots of stub functions for every entry point. I guess we could have a script to generate them given a header, but still ...
thanks -mike
Mike Hearn mike@navi.cx writes:
They also make adding new DLLs easier, as you don't have to submit lots of stub functions for every entry point. I guess we could have a script to generate them given a header, but still ...
Not adding the functions at all is even easier, and the results are pretty much the same... As you noted, in general the only advantage of stubs is that you get a better error message, but that would be fairly easy to handle at the loader level. The problem with adding stubs everywhere is that now we don't know which ones are really needed.
On Tue, 2004-12-14 at 15:41 +0100, Alexandre Julliard wrote:
Not adding the functions at all is even easier, and the results are pretty much the same... As you noted, in general the only advantage of stubs is that you get a better error message, but that would be fairly easy to handle at the loader level. The problem with adding stubs everywhere is that now we don't know which ones are really needed.
Hmm, you mean we could remove the spec file entries that apps never actually link against?
Perhaps the first step then would be to implement support in winebuild and the loader such that the list of stubbed functions is exported then the loader can link imports from those to stubs generated on the fly. At that point the winebuild support for stubs could be removed, and all the @ stub entries also deleted in one go.
thanks -mike
Mike Hearn wrote:
On Tue, 2004-12-14 at 15:41 +0100, Alexandre Julliard wrote:
Not adding the functions at all is even easier, and the results are pretty much the same... As you noted, in general the only advantage of stubs is that you get a better error message, but that would be fairly easy to handle at the loader level. The problem with adding stubs everywhere is that now we don't know which ones are really needed.
Hmm, you mean we could remove the spec file entries that apps never actually link against?
I would like to remove the Callback* stubs in kernel32, that aren't linked to by any apps and aren't present in the WinNT kernel32.
Perhaps the first step then would be to implement support in winebuild and the loader such that the list of stubbed functions is exported then the loader can link imports from those to stubs generated on the fly. At that point the winebuild support for stubs could be removed, and all the @ stub entries also deleted in one go.
I think that's a step backwards. Look, for example, in shlwapi or shdocvw. There are stubs there that are exported by ordinal only in the native dll and so removing them would lose the name of the function. Not to mention that I've fixed one bug with nasty stack corruption caused by missing ordinal exports. Also, I see the stubs in the .spec file as a really broad todo list. A stub in a dll that I am interested in working on tells me to look up documentation for that function and write a test for it, then implement it. I don't need an obscure application to let me know that it is unimplemented. IMHO, the Wine project should be moving away from just-in-time implementing of functions and moving towards implementing *everything* (with some obvious exceptions). This is the only way a high percentage of applications will work on Wine.
Rob
On Tue, Dec 14, 2004 at 10:16:29AM -0600, Robert Shearman wrote:
IMHO, the Wine project should be moving away from just-in-time implementing of functions and moving towards implementing *everything* (with some obvious exceptions). This is the only way a high percentage of applications will work on Wine.
I second that, especially for the visible (UI elements). Otherwise we will always have ugly artifacts that make apps run under wine look rather unprofessional/unpleasant. At best, the user doesn't get a worm and fuzzy feeling. In reality, we're losing (quite a bit of) mindshare.
On Tue, 2004-12-14 at 12:01 -0500, Dimitrie O. Paun wrote:
On Tue, Dec 14, 2004 at 10:16:29AM -0600, Robert Shearman wrote:
IMHO, the Wine project should be moving away from just-in-time implementing of functions and moving towards implementing *everything* (with some obvious exceptions). This is the only way a high percentage of applications will work on Wine.
I second that, especially for the visible (UI elements). Otherwise we will always have ugly artifacts that make apps run under wine look rather unprofessional/unpleasant. At best, the user doesn't get a worm and fuzzy feeling. In reality, we're losing (quite a bit of) mindshare.
UI polish is definitely important, but I think just-in-time development is our only choice right now. If keen hackers were lining up at the door desperate to write patches then yeah, maybe we could rethink, but as it is new people who just write general patches (ie, aren't just interested in one specific app or DLL) join the project fairly rarely. Telling people to just implement random functions that may be called by one in ten thousand apps doesn't make any sense.
The widget library is one exception to that I think: the rebar control is horribly busted right now but not quite busted enough to actually crash popular apps, with the result that nobody works on it. So improving the code for its own sake makes sense there.
thanks -mike
Robert Shearman rob@codeweavers.com writes:
Mike Hearn wrote:
Perhaps the first step then would be to implement support in winebuild and the loader such that the list of stubbed functions is exported then the loader can link imports from those to stubs generated on the fly. At that point the winebuild support for stubs could be removed, and all the @ stub entries also deleted in one go.
I think that's a step backwards. Look, for example, in shlwapi or shdocvw. There are stubs there that are exported by ordinal only in the native dll and so removing them would lose the name of the function. Not to mention that I've fixed one bug with nasty stack corruption caused by missing ordinal exports.
Obviously we can't blindly remove all stubs, each individual case needs to be looked at. But there aren't that many ordinal exports, and adding dummy functions for them shouldn't be that hard.
Also, I see the stubs in the .spec file as a really broad todo list. A stub in a dll that I am interested in working on tells me to look up documentation for that function and write a test for it, then implement it. I don't need an obscure application to let me know that it is unimplemented.
Yes but that doesn't require winebuild support, a commented out entry in the spec file can serve the same purpose.
IMHO, the Wine project should be moving away from just-in-time implementing of functions and moving towards implementing *everything* (with some obvious exceptions). This is the only way a high percentage of applications will work on Wine.
Replacing stubs by real functions is a step in that direction IMO. At least this way we'll have prototype information that will make it more likely that someone starts working on the function.
Hi,
On Tue, Dec 14, 2004 at 11:47:11AM +0100, Ge van Geldorp wrote:
From: julliard@winehq.org
I don't think we want to add a special mode just for stubs; they should really be replaced by proper functions (even if the functions are stubs themselves, at least they can print the parameters and try to return something meaningful instead of killing the app). You can view the lack of stubs support on PE as an encouragement to help us remove them from the spec files ;-)
Seems impossible for functions with unknown calling conventions and number of parameters.
What's the problem here? If you have an app exercising that function, then you have an object ready for examination already, and if you don't have any app using it, then you don't have any problem anyway... OK, mere mortal endusers might have problems with some apps crashing, but they're easily lost in slightly more severe cases anyway ;)
Andreas Mohr
"Ge van Geldorp" gvg@reactos.com wrote:
I don't think we want to add a special mode just for stubs; they should really be replaced by proper functions (even if the functions are stubs themselves, at least they can print the parameters and try to return something meaningful instead of killing the app). You can view the lack of stubs support on PE as an encouragement to help us remove them from the spec files ;-)
Seems impossible for functions with unknown calling conventions and number of parameters.
In that case you can remove those (only a few) stubs from the .spec files, but most of the stubs are well documented APIs.