Re: Use Get/SetThreadErrorMode if it is available (on Windows 7 and above).

[Peter Rosin]

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  <address@hidden>

        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?

thread-error-mode-2.patch
Description: Text document