On 6/8/20 5:59 PM, Alex Henrie wrote:
On Sun, Jun 7, 2020 at 11:17 PM Zebediah Figura z.figura12@gmail.com wrote:
On 6/7/20 11:41 PM, Alex Henrie wrote:
Signed-off-by: Alex Henrie alexhenrie24@gmail.com
This will help tremendously with debugging null pointer segfaults.
Seconded wholeheartedly. I've kept a similar patch in my local tree for quite a while.
Even worse, applications will detect that a function is missing but silently fail...
Glad you approve!
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index d1b71efc07..2c37135105 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1865,6 +1865,17 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name, }
RtlLeaveCriticalSection( &loader_section );
- if (__WINE_GET_DEBUGGING_WARN( __wine_dbch___default ) && module && (name || ord) && !*address)
Why not just WARN_ON(module)?
I didn't know that you could do that. Thanks for the tip!
Couldn't we just check for STATUS_PROCEDURE_NOT_FOUND? (I also wouldn't think it's worth checking that the parameters are valid.)
Yes, I realized after I sent v1 that the function should be checking ret instead of address. I put in some argument checks because I didn't want to see noise from this function when the failure is not a bug in Wine. Do you see any strong reason to drop the argument checks?
Mostly because I don't expect much of said noise, but I don't feel strongly about it.
- {
WCHAR path_buffer[MAX_PATH];UNICODE_STRING module_path = { 0, sizeof(path_buffer), path_buffer };path_buffer[0] = 0;LdrGetDllFullName( module, &module_path );WARN( "function %s (ordinal %d) not found in module %s\n",wine_dbgstr_a(name ? name->Buffer : NULL), ord, wine_dbgstr_w(path_buffer) );As long as we're calling get_modref() in this function anyway, if we're concerned about printing the module name, I think it makes sense to just access the WINE_MODREF directly. That way you wouldn't have to possibly truncate the buffer (not that it probably matters).
You're right, I really didn't need to use LdrGetDllFullName. I will send a new patch that does not depend on it.
-Alex