Am 07.01.2024 um 15:46 schrieb Eli Zaretskii:
[I re-added the other addressees, as I don' think you meant to make
this discussion private between the two of us.]


Yeah, that was a mistake.

Date: Sun, 7 Jan 2024 12:58:29 +0100
From: Björn Schäpers <g...@hazardy.de>

Am 07.01.2024 um 07:50 schrieb Eli Zaretskii:
Date: Sat, 6 Jan 2024 23:15:24 +0100
From: Björn Schäpers <g...@hazardy.de>
Cc: gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org

This patch adds libraries which are loaded after backtrace_initialize, like
plugins or similar.

I don't know what style is preferred for the Win32 typedefs, should the code use
PVOID or void*?

It doesn't matter, at least not if the source file includes the
Windows header files (where PVOID is defined).

+  if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1)

IMO, it would be better to supply a #define if undefined:

#ifndef LDR_DLL_NOTIFICATION_REASON_LOADED
# define LDR_DLL_NOTIFICATION_REASON_LOADED 1
#endif


I surely can define it. But the ifndef is not needed, since there are no headers
containing the function signatures, structures or the defines:
https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification

OK, I wasn't sure about that.

+  if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
+                         | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+                         (TCHAR*) notification_data->dll_base,

Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies
on compile-time definition of UNICODE?  (I'm not familiar with the
internals of libbacktrace, so apologies if this is a silly question.)

Thanks.

As far as I can see it's the first time for TCHAR, I would've gone for
GetModuleHandleExW, but 
https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html

That was about GetModuleHandle, not about GetModuleHandleEx.  For the
latter, all Windows versions that support it also support "wide" APIs.
So my suggestion is to use GetModuleHandleExW here.  However, you will
need to make sure that notification_data->dll_base is declared as
'wchar_t *', not 'char *'.  If dll_base is declared as 'char *', then
only GetModuleHandleExA will work, and you will lose the ability to
support file names with non-ASCII characters outside of the current
system codepage.

The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag GetModuleHandleEx does not look for a name, but uses an adress in the module to get the HMODULE, so you cast it to char* or wchar_t* depending on which function you call. Actually one could just cast the dll_base to HMODULE, at least in win32 on x86 the HMODULE of a dll is always its base adress. But to make it safer and future proof I went the way through GetModuleHandeEx.


But I didn't want to force GetModuleHandleExA, so I went for TCHAR and
GetModuleHandleEx so it automatically chooses which to use. Same for
GetModuleHandle of ntdll.dll.

The considerations for GetModuleHandle and for GetModuleHandleEx are
different: the former is also available on old versions of Windows
that doesn't support "wide" APIs.

Reply via email to