On Mon, Apr 28, 2008 at 3:51 AM, Steven Edwards winehacker@gmail.com wrote:
On Sun, Apr 27, 2008 at 9:19 AM, Dan Kegel dank@kegel.com wrote:
- You never clear that environment variable... what happens
when an app just tests for the existence of a DLL by trying to load it? We don't want to print an error then. (That's why I originally suggested clearing the env var after the app finished loading. That's still broken -- it won't catch errors in helper exes -- but it's better than nothing.)
I assume you mean if the application is just checking manually for the dll via GetModuleHandle or LoadLibrary? I'll write a test case for this to see if it causes a problem. My assumption was that the failure case for that was at a higher level but I'll check with a test case. I don't think we need to clear the variable....what I mean is I wonder if there is a way to set the variable to be inherited by all child processes launched by start.exe.so in unix mode. I'll look in to this as well.
I assumed the code path must be different so I wrote a test app that tried to do a GetProcAddress or LoadLibrary on a non-existent dll and was right, It did not throw the messagebox. So that just leaves the issue of do we want to clear the environment for the child process. My thinking is no, because I assume most applications that invoke other applications via CreateProcess inherit the environment of the parent just like apps started from start.exe.CreateProcess do. It would not really make since for a vendor to have an application foo.exe that calls bar.exe to be randomly setting and unsetting variables in the package so I assume then we want the verbose variable to remain resident for all child processes started by start.exe in case foo.exe calls bar.exe which is linked to a missing dll so the user will get a warning rather than just failing silently. Unless you can point me to a case otherwise I think that it is correct behavior.
IMHO if we can extend this patch a bit to return more information via the message box such as the name of the missing dll and also address the case of dlls that are there but with missing exports, then this should go in before the freeze so we have time to shake it down. If worse comes to worse we can revert it. I think the behavior as it stands with this patch is better than a user clicking on a menu or *.exe from KDE/GNOME and nothing happening at all.
Thanks Steven