Hi Ralf!
Den 2010-03-16 22:43 skrev Ralf Wildenhues:
I cannot see big problems with this patch from looking over it.
I think GCS wants no whitespace after opening parentheses, you could
line-wrap before '='.
I fixed that (and updated the changelog date), thanks for looking!
When you use GetProcAdress in the setup function, you may be setting the
error that can be read with GetLastError. Is there a chance the code
(since your last patch) interferes with a previous error set there?
I don't see that. If you by the setup function refer to
wrap_setthreaderrormode, then that function is only called before the
"real" LoadLibrary call. After the "real" LoadLibrary call the wrapper
has done its job and is not used again. Instead the unwrapped
SetThreadErrorMode or the fallback function (which just calls
SetErrorMode) is called, and both of those are not at all likely to
fail when given a previously valid error mode as is the case here.
So, if the "real" LoadLibrary fails it is responsible for calling
SetLastError and the followup SetThreadErrorMode/SetErrorMode is
extremely unlikely to fail. If LoadLibrary succeeds, the application
might observe that GetLastError changes over a ltdl call (due to
GetProcAddress failing in one of the wrap_* functions), but relying
on it to not change seems fragile so I don't think we should care.
Windows is not always careful about not touching the last error on
success, so I don't see why we should be more careful and restore
the last error with a final call to SetLastError. Microsoft claims
that MSDN states when a function clobbers the last error on success,
but MSDN is not always correct in details like that...
Hmm, my last argument made me think that it's perhaps not wise for
libtool to rely on the last error to not change over a successful
call either. But that potential problem was there before this patch
too, so I'm proposing a separate patch for that in a followup mail...
So, ok to push this as attached?
Cheers,
Peter
2010-03-17 Peter Rosin <p...@lysator.liu.se>
Use Get/SetThreadErrorMode if they are available.
* libltdl/loaders/loadlibrary.c (wrap_geterrormode): Replaced...
(wrap_getthreaderrormode): ...by this function that checks
first for GetThreadErrorMode, then GetErrorMode and makes use
of either of those or...
(fallback_getthreaderrormode): ...else falls back to this
replacement function that implements the old workaround, which
was previously implemented in...
(fallback_geterrormode): ...this now renamed function.
(geterrormode): Replaced...
(getthreaderrormode): ...by this function pointer that points
at either of wrap_getthreaderrormode, GetThreadErrorMode,
GetErrorMode or fallback_getthreaderrormode.
(wrap_setthreaderrormode): New function that checks if
SetThreadErrorMode is supported by the system and makes use of
it if it is.
(fallback_setthreaderrormode): New function that is used
otherwise that implements the old version using SetErrorMode.
(setthreaderrormode): New function pointer that points at
either of wrap_setthreaderrormode, SetThreadErrorMode or
fallback_setthreaderrormode.
(vm_open): Adjust to the above.
--
They are in the crowd with the answer before the question.
> Why do you dislike Jeopardy?
diff --git a/libltdl/loaders/loadlibrary.c b/libltdl/loaders/loadlibrary.c
index 139851a..620e7cf 100644
--- a/libltdl/loaders/loadlibrary.c
+++ b/libltdl/loaders/loadlibrary.c
@@ -104,12 +104,16 @@ get_vtable (lt_user_data loader_data)
#define LOADLIB_SETERROR(errcode) LOADLIB__SETERROR (LT__STRERROR (errcode))
static const char *loadlibraryerror (const char *default_errmsg);
-static UINT WINAPI wrap_geterrormode (void);
-static UINT WINAPI fallback_geterrormode (void);
+static DWORD WINAPI wrap_getthreaderrormode (void);
+static DWORD WINAPI fallback_getthreaderrormode (void);
+static BOOL WINAPI wrap_setthreaderrormode (DWORD mode, DWORD *oldmode);
+static BOOL WINAPI fallback_setthreaderrormode (DWORD mode, DWORD *oldmode);
-typedef UINT (WINAPI geterrormode_type) (void);
+typedef DWORD (WINAPI getthreaderrormode_type) (void);
+typedef BOOL (WINAPI setthreaderrormode_type) (DWORD, DWORD *);
-static geterrormode_type *geterrormode = wrap_geterrormode;
+static getthreaderrormode_type *getthreaderrormode = wrap_getthreaderrormode;
+static setthreaderrormode_type *setthreaderrormode = wrap_setthreaderrormode;
static char *error_message = 0;
@@ -187,13 +191,13 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char
*filename,
{
/* Silence dialog from LoadLibrary on some failures. */
- UINT errormode = geterrormode ();
- SetErrorMode (errormode | SEM_FAILCRITICALERRORS);
+ DWORD errormode = getthreaderrormode ();
+ setthreaderrormode (errormode | SEM_FAILCRITICALERRORS, NULL);
module = LoadLibrary (wpath);
/* Restore the error mode. */
- SetErrorMode (errormode);
+ setthreaderrormode (errormode, NULL);
}
/* libltdl expects this function to fail if it is unable
@@ -298,28 +302,64 @@ loadlibraryerror (const char *default_errmsg)
return len ? error_message : default_errmsg;
}
-/* A function called through the geterrormode variable which checks
- if the system supports GetErrorMode and arranges for it or a
- fallback implementation to be called directly in the future. The
- selected version is then called. */
-static UINT WINAPI
-wrap_geterrormode (void)
+/* A function called through the getthreaderrormode variable which checks
+ if the system supports GetThreadErrorMode (or GetErrorMode) and arranges
+ for it or a fallback implementation to be called directly in the future.
+ The selected version is then called. */
+static DWORD WINAPI
+wrap_getthreaderrormode (void)
{
HMODULE kernel32 = GetModuleHandleA ("kernel32.dll");
- geterrormode = (geterrormode_type *) GetProcAddress (kernel32,
- "GetErrorMode");
- if (!geterrormode)
- geterrormode = fallback_geterrormode;
- return geterrormode ();
+ getthreaderrormode
+ = (getthreaderrormode_type *) GetProcAddress (kernel32,
+ "GetThreadErrorMode");
+ if (!getthreaderrormode)
+ getthreaderrormode
+ = (getthreaderrormode_type *) GetProcAddress (kernel32,
+ "GetErrorMode");
+ if (!getthreaderrormode)
+ getthreaderrormode = fallback_getthreaderrormode;
+ return getthreaderrormode ();
}
-/* A function called through the geterrormode variable for cases
- where the system does not support GetErrorMode */
-static UINT WINAPI
-fallback_geterrormode (void)
+/* A function called through the getthreaderrormode variable for cases
+ where the system does not support GetThreadErrorMode or GetErrorMode */
+static DWORD WINAPI
+fallback_getthreaderrormode (void)
{
/* Prior to Windows Vista, the only way to get the current error
mode was to set a new one. In our case, we are setting a new
- error mode right after "getting" it, so that's fairly ok. */
- return SetErrorMode (SEM_FAILCRITICALERRORS);
+ error mode right after "getting" it while ignoring the error
+ mode in effect when setting the new error mode, so that's
+ fairly ok. */
+ return (DWORD) SetErrorMode (SEM_FAILCRITICALERRORS);
+}
+
+/* A function called through the setthreaderrormode variable which checks
+ if the system supports SetThreadErrorMode and arranges for it or a
+ fallback implementation to be called directly in the future.
+ The selected version is then called. */
+static BOOL WINAPI
+wrap_setthreaderrormode (DWORD mode, DWORD *oldmode)
+{
+ HMODULE kernel32 = GetModuleHandleA ("kernel32.dll");
+ setthreaderrormode
+ = (setthreaderrormode_type *) GetProcAddress (kernel32,
+ "SetThreadErrorMode");
+ if (!setthreaderrormode)
+ setthreaderrormode = fallback_setthreaderrormode;
+ return setthreaderrormode (mode, oldmode);
+}
+
+/* A function called through the setthreaderrormode variable for cases
+ where the system does not support SetThreadErrorMode. */
+static BOOL WINAPI
+fallback_setthreaderrormode (DWORD mode, DWORD *oldmode)
+{
+ /* Prior to Windows 7, there was no way to set the thread local error
+ mode, so set the process global error mode instead. */
+ DWORD old = (DWORD) SetErrorMode (mode);
+ if (oldmode)
+ *oldmode = old;
+ return TRUE;
}
