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
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. */
-
