Re[2]: [lmi] trivial patch for LoadLibrary() call

[Vadim Zeitlin]

On Wed, 31 Dec 2008 02:49:43 +0000 Greg Chicares <address@hidden> wrote:

GC> I think I'd rather wait for the other changes and apply them
GC> all together.

 Ok, I just wanted to keep separate things in separate patches.

GC> BTW, should these:
GC> 
GC> main_wx.cpp:    ::SendMessage(reinterpret_cast<HWND>(w.GetHandle()), 
WM_PASTE, 0, 0);
GC> msw_workarounds.cpp:    if(0 == 
::FreeLibrary(::GetModuleHandle(dll_name.c_str())))
GC> system_command_non_wx.cpp:    ::CreateProcess
GC> system_command_non_wx.cpp:    ::CloseHandle(process_info.hThread);
GC> system_command_non_wx.cpp:    ::WaitForSingleObject(process_info.hProcess, 
INFINITE);
GC> system_command_non_wx.cpp:    ::GetExitCodeProcess(process_info.hProcess, 
&exit_code);
GC> system_command_non_wx.cpp:    ::CloseHandle(process_info.hProcess);
GC> 
GC> be changed for the same reason as LoadLibrary[A]?

 Some of these functions (CloseHandle(), FreeLibrary(), WaitForSingleObject(),
GetExitCodeProcess()) don't have A/W-suffixed versions at all because they
don't take any string parameters so they definitely shouldn't.
GetModuleHandle() should and I have no idea why didn't the compiler give me
an error message about it and only the one about about LoadLibrary() -- of
course, after correcting the LoadLibrary() call it does fail to compile
GetModuleHandle() one, so the full change to this file is:

--- msw_workarounds.cpp 2008-12-27 02:57:00 +0000
+++ msw_workarounds.cpp 2008-12-31 10:56:50 +0000
@@ -90,7 +90,7 @@
 #ifdef LMI_MSW
     fenv_initialize();

-    if(0 == ::LoadLibrary(dll_name.c_str()))
+    if(0 == ::LoadLibraryA(dll_name.c_str()))
         {
         std::ostringstream oss;
         oss << "Failed to preload '" << dll_name << "'.";
@@ -118,7 +118,7 @@
 void MswDllPreloader::UnloadOneDll(std::string const& dll_name)
 {
 #ifdef LMI_MSW
-    if(0 == ::FreeLibrary(::GetModuleHandle(dll_name.c_str())))
+    if(0 == ::FreeLibrary(::GetModuleHandleA(dll_name.c_str())))
         {
         std::ostringstream oss;
         oss << "Failed to unload '" << dll_name << "'.";


 This leaves SendMessage(): it does have A/W versions but which one is
called only matters when strings are used as its arguments in this case are
not strings (they're in fact unused) so we can keep it as is for
simplicity.

 Finally CreateProcess() in system_command_non_wx.cpp is a special case: as
this file doesn't use wx, it doesn't get UNICODE defined before including
<windows.h> and so here CreateProcess() is actually already defined as
CreateProcessA() and so everything works or at least compiles fine. It
seems dangerous to have different definitions of Windows functions in
different translation units but OTOH I don't see any real harm coming out
of this (in particular they're just macros so there are no issues with ODR
or anything like that). It can be rather confusing that LoadLibrary() needs
a char string in one file and a wide string in another one though so I
think I'd prefer to use CreateProcessA() for clarify even though it happens
to not be necessary here.

 Regards,
VZ