[PATCH 0/5] MR1543: mmdevapi: Handle process_attach, process_detach and test_connect UNIX driver functions
The goal is to eventually move all interfaces from each driver's `mmdevdrv.c` into `mmdevapi`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
From: Davide Beatrici <git(a)davidebeatrici.dev> Required for the NTSTATUS definition. The other types, such as UINT64, are also guaranteed to be defined. --- include/wine/unixlib.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/wine/unixlib.h b/include/wine/unixlib.h index fcb25422524..36f05c70ba6 100644 --- a/include/wine/unixlib.h +++ b/include/wine/unixlib.h @@ -21,6 +21,8 @@ #ifndef __WINE_WINE_UNIXLIB_H #define __WINE_WINE_UNIXLIB_H +#include "winternl.h" + typedef UINT64 unixlib_handle_t; extern NTSTATUS WINAPI __wine_unix_call( unixlib_handle_t handle, unsigned int code, void *args ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
From: Davide Beatrici <git(a)davidebeatrici.dev> Previously, an attempted NULL call returned the very generic STATUS_ACCESS_VIOLATION. --- dlls/ntdll/unix/loader.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index deb25a8e35e..14fe1229723 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -1329,7 +1329,11 @@ NTSTATUS ntdll_init_syscalls( ULONG id, SYSTEM_SERVICE_TABLE *table, void **disp */ NTSTATUS WINAPI __wine_unix_call( unixlib_handle_t handle, unsigned int code, void *args ) { - return ((unixlib_entry_t*)(UINT_PTR)handle)[code]( args ); + unixlib_entry_t *funcs = (unixlib_entry_t*)(UINT_PTR)handle; + if (funcs[code]) + return funcs[code](args); + + return STATUS_NOT_IMPLEMENTED; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
From: Davide Beatrici <git(a)davidebeatrici.dev> In preparation for calling the audio driver's UNIX functions directly. --- dlls/mmdevapi/main.c | 20 ++++++++++++++------ dlls/mmdevapi/mmdevapi.h | 2 ++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/dlls/mmdevapi/main.c b/dlls/mmdevapi/main.c index c792661af33..43fc03f3aa3 100644 --- a/dlls/mmdevapi/main.c +++ b/dlls/mmdevapi/main.c @@ -65,23 +65,28 @@ static const char *get_priority_string(int prio) static BOOL load_driver(const WCHAR *name, DriverFuncs *driver) { - WCHAR driver_module[264]; + NTSTATUS status; + WCHAR driver_module[264]; lstrcpyW(driver_module, L"wine"); lstrcatW(driver_module, name); lstrcatW(driver_module, L".drv"); TRACE("Attempting to load %s\n", wine_dbgstr_w(driver_module)); - driver->module = LoadLibraryW(driver_module); - if(!driver->module){ - TRACE("Unable to load %s: %lu\n", wine_dbgstr_w(driver_module), - GetLastError()); + if (!(driver->module = LoadLibraryW(driver_module))) { + ERR("Unable to load %s: %lu\n", wine_dbgstr_w(driver_module), GetLastError()); return FALSE; } + if ((status = NtQueryVirtualMemory(GetCurrentProcess(), driver->module, MemoryWineUnixFuncs, + &driver->module_unixlib, sizeof(driver->module_unixlib), NULL))) { + ERR("Unable to load UNIX functions: %lx\n", status); + goto fail; + } + #define LDFC(n) do { driver->p##n = (void*)GetProcAddress(driver->module, #n);\ - if(!driver->p##n) { FreeLibrary(driver->module); return FALSE; } } while(0) + if(!driver->p##n) { goto fail; } } while(0) LDFC(GetPriority); LDFC(GetEndpointIDs); LDFC(GetAudioEndpoint); @@ -98,6 +103,9 @@ static BOOL load_driver(const WCHAR *name, DriverFuncs *driver) wine_dbgstr_w(driver_module), get_priority_string(driver->priority)); return TRUE; +fail: + FreeLibrary(driver->module); + return FALSE; } static BOOL WINAPI init_driver(INIT_ONCE *once, void *param, void **context) diff --git a/dlls/mmdevapi/mmdevapi.h b/dlls/mmdevapi/mmdevapi.h index d694a3930aa..410d958bf46 100644 --- a/dlls/mmdevapi/mmdevapi.h +++ b/dlls/mmdevapi/mmdevapi.h @@ -19,12 +19,14 @@ #include "unixlib.h" #include <wine/list.h> +#include <wine/unixlib.h> extern HRESULT MMDevEnum_Create(REFIID riid, void **ppv) DECLSPEC_HIDDEN; extern void MMDevEnum_Free(void) DECLSPEC_HIDDEN; typedef struct _DriverFuncs { HMODULE module; + unixlib_handle_t module_unixlib; WCHAR module_name[64]; int priority; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/mmdevapi/main.c | 19 ++++++++++++++++--- dlls/winepulse.drv/mmdevdrv.c | 4 +--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dlls/mmdevapi/main.c b/dlls/mmdevapi/main.c index 43fc03f3aa3..e67c8f8b10a 100644 --- a/dlls/mmdevapi/main.c +++ b/dlls/mmdevapi/main.c @@ -19,7 +19,9 @@ #include <stdarg.h> +#include "ntstatus.h" #define COBJMACROS +#define WIN32_NO_STATUS #include "windef.h" #include "winbase.h" #include "wingdi.h" @@ -85,6 +87,12 @@ static BOOL load_driver(const WCHAR *name, DriverFuncs *driver) goto fail; } + if ((status = __wine_unix_call(driver->module_unixlib, process_attach, NULL)) && + status != STATUS_NOT_IMPLEMENTED) { + ERR("Unable to initialize library: %lx\n", status); + goto fail; + } + #define LDFC(n) do { driver->p##n = (void*)GetProcAddress(driver->module, #n);\ if(!driver->p##n) { goto fail; } } while(0) LDFC(GetPriority); @@ -175,9 +183,14 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) DisableThreadLibraryCalls(hinstDLL); break; case DLL_PROCESS_DETACH: - if(lpvReserved) - break; - MMDevEnum_Free(); + if (drvs.module_unixlib) { + const NTSTATUS status = __wine_unix_call(drvs.module_unixlib, process_detach, NULL); + if (status && status != STATUS_NOT_IMPLEMENTED) + WARN("Unable to deinitialize library: %lx\n", status); + } + + if (!lpvReserved) + MMDevEnum_Free(); break; } diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 5de9a941099..99dbe24c82f 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -87,14 +87,12 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) DisableThreadLibraryCalls(dll); if (__wine_init_unix_call()) return FALSE; - if (WINE_UNIX_CALL(process_attach, NULL)) - return FALSE; } else if (reason == DLL_PROCESS_DETACH) { struct device_cache *device, *device_next; LIST_FOR_EACH_ENTRY_SAFE(device, device_next, &g_devices_cache, struct device_cache, entry) free(device); - WINE_UNIX_CALL(process_detach, NULL); + if (pulse_thread) { WaitForSingleObject(pulse_thread, INFINITE); CloseHandle(pulse_thread); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/mmdevapi/main.c | 46 ++++++++++++++++++- dlls/mmdevapi/mmdevapi.h | 5 +- dlls/winealsa.drv/mmdevdrv.c | 5 -- dlls/winealsa.drv/winealsa.drv.spec | 1 - dlls/winecoreaudio.drv/mmdevdrv.c | 5 -- dlls/winecoreaudio.drv/winecoreaudio.drv.spec | 1 - dlls/wineoss.drv/mmdevdrv.c | 11 ----- dlls/wineoss.drv/wineoss.drv.spec | 1 - dlls/winepulse.drv/mmdevdrv.c | 11 ----- dlls/winepulse.drv/winepulse.drv.spec | 1 - 10 files changed, 46 insertions(+), 41 deletions(-) diff --git a/dlls/mmdevapi/main.c b/dlls/mmdevapi/main.c index e67c8f8b10a..a148555ca22 100644 --- a/dlls/mmdevapi/main.c +++ b/dlls/mmdevapi/main.c @@ -18,6 +18,7 @@ */ #include <stdarg.h> +#include <wchar.h> #include "ntstatus.h" #define COBJMACROS @@ -50,6 +51,26 @@ DriverFuncs drvs; const WCHAR drv_keyW[] = L"Software\\Wine\\Drivers"; +static char *get_test_connect_name(void) +{ + WCHAR path[MAX_PATH], *name; + size_t len; + char *str; + + GetModuleFileNameW(NULL, path, ARRAY_SIZE(path)); + + name = wcsrchr(path, '\\'); + name = name ? name + 1 : path; + + len = WideCharToMultiByte(CP_UTF8, 0, name, -1, NULL, 0, NULL, NULL); + if (!(str = malloc(len))) + return NULL; + + WideCharToMultiByte(CP_UNIXCP, 0, name, -1, str, len, NULL, NULL); + + return str; +} + static const char *get_priority_string(int prio) { switch(prio){ @@ -95,7 +116,6 @@ static BOOL load_driver(const WCHAR *name, DriverFuncs *driver) #define LDFC(n) do { driver->p##n = (void*)GetProcAddress(driver->module, #n);\ if(!driver->p##n) { goto fail; } } while(0) - LDFC(GetPriority); LDFC(GetEndpointIDs); LDFC(GetAudioEndpoint); LDFC(GetAudioSessionManager); @@ -104,7 +124,29 @@ static BOOL load_driver(const WCHAR *name, DriverFuncs *driver) /* optional - do not fail if not found */ driver->pGetPropValue = (void*)GetProcAddress(driver->module, "GetPropValue"); - driver->priority = driver->pGetPriority(); + { + struct test_connect_params params; + char *name = get_test_connect_name(); + params.name = name; + + status = __wine_unix_call(driver->module_unixlib, test_connect, ¶ms); + + free(name); + + switch (status) + { + case STATUS_SUCCESS: + driver->priority = params.priority; + break; + case STATUS_NOT_IMPLEMENTED: + driver->priority = Priority_Neutral; + break; + default: + ERR("Unable to retrieve driver priority: %lx\n", status); + goto fail; + } + } + lstrcpyW(driver->module_name, driver_module); TRACE("Successfully loaded %s with priority %s\n", diff --git a/dlls/mmdevapi/mmdevapi.h b/dlls/mmdevapi/mmdevapi.h index 410d958bf46..bfbcc4ad96c 100644 --- a/dlls/mmdevapi/mmdevapi.h +++ b/dlls/mmdevapi/mmdevapi.h @@ -28,13 +28,12 @@ typedef struct _DriverFuncs { HMODULE module; unixlib_handle_t module_unixlib; WCHAR module_name[64]; - int priority; - /* Returns a "priority" value for the driver. Highest priority wins. + /* Highest priority wins. * If multiple drivers think they are valid, they will return a * priority value reflecting the likelihood that they are actually * valid. See enum _DriverPriority. */ - int (WINAPI *pGetPriority)(void); + int priority; /* ids gets an array of human-friendly endpoint names * keys gets an array of driver-specific stuff that is used diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index ea48143005b..fb81faf9d00 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -212,11 +212,6 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) return TRUE; } -int WINAPI AUDDRV_GetPriority(void) -{ - return Priority_Neutral; -} - static HRESULT alsa_stream_release(stream_handle stream, HANDLE timer_thread) { struct release_stream_params params; diff --git a/dlls/winealsa.drv/winealsa.drv.spec b/dlls/winealsa.drv/winealsa.drv.spec index bd83ea2e765..d846ade8ce2 100644 --- a/dlls/winealsa.drv/winealsa.drv.spec +++ b/dlls/winealsa.drv/winealsa.drv.spec @@ -4,7 +4,6 @@ @ stdcall -private modMessage(long long long long long) ALSA_modMessage # MMDevAPI driver functions -@ stdcall -private GetPriority() AUDDRV_GetPriority @ stdcall -private GetEndpointIDs(long ptr ptr ptr ptr) AUDDRV_GetEndpointIDs @ stdcall -private GetAudioEndpoint(ptr ptr ptr) AUDDRV_GetAudioEndpoint @ stdcall -private GetAudioSessionManager(ptr ptr) AUDDRV_GetAudioSessionManager diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c index 367f7ba0ab8..877c1f7c3bc 100644 --- a/dlls/winecoreaudio.drv/mmdevdrv.c +++ b/dlls/winecoreaudio.drv/mmdevdrv.c @@ -213,11 +213,6 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) return TRUE; } -int WINAPI AUDDRV_GetPriority(void) -{ - return Priority_Neutral; -} - static void set_device_guid(EDataFlow flow, HKEY drv_key, const WCHAR *key_name, GUID *guid) { diff --git a/dlls/winecoreaudio.drv/winecoreaudio.drv.spec b/dlls/winecoreaudio.drv/winecoreaudio.drv.spec index 9d9f781cf4e..1dfbfb79eb9 100644 --- a/dlls/winecoreaudio.drv/winecoreaudio.drv.spec +++ b/dlls/winecoreaudio.drv/winecoreaudio.drv.spec @@ -4,7 +4,6 @@ @ stdcall -private modMessage(long long long long long) CoreAudio_modMessage # MMDevAPI driver functions -@ stdcall -private GetPriority() AUDDRV_GetPriority @ stdcall -private GetEndpointIDs(long ptr ptr ptr ptr) AUDDRV_GetEndpointIDs @ stdcall -private GetAudioEndpoint(ptr ptr ptr) AUDDRV_GetAudioEndpoint @ stdcall -private GetAudioSessionManager(ptr ptr) AUDDRV_GetAudioSessionManager diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c index 7411f87a4d7..6993fa88d0a 100644 --- a/dlls/wineoss.drv/mmdevdrv.c +++ b/dlls/wineoss.drv/mmdevdrv.c @@ -226,17 +226,6 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) return TRUE; } -int WINAPI AUDDRV_GetPriority(void) -{ - struct test_connect_params params; - - params.name = NULL; - - OSS_CALL(test_connect, ¶ms); - - return params.priority; -} - static HRESULT stream_release(stream_handle stream, HANDLE timer_thread) { struct release_stream_params params; diff --git a/dlls/wineoss.drv/wineoss.drv.spec b/dlls/wineoss.drv/wineoss.drv.spec index fe6cc518c33..a093649e982 100644 --- a/dlls/wineoss.drv/wineoss.drv.spec +++ b/dlls/wineoss.drv/wineoss.drv.spec @@ -5,7 +5,6 @@ @ stdcall -private modMessage(long long long long long) OSS_modMessage # MMDevAPI driver functions -@ stdcall -private GetPriority() AUDDRV_GetPriority @ stdcall -private GetEndpointIDs(long ptr ptr ptr ptr) AUDDRV_GetEndpointIDs @ stdcall -private GetAudioEndpoint(ptr ptr ptr) AUDDRV_GetAudioEndpoint @ stdcall -private GetAudioSessionManager(ptr ptr) AUDDRV_GetAudioSessionManager diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 99dbe24c82f..b8151956c14 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -478,17 +478,6 @@ end: return params.result; } -int WINAPI AUDDRV_GetPriority(void) -{ - struct test_connect_params params; - char *name; - - params.name = name = get_application_name(FALSE); - pulse_call(test_connect, ¶ms); - free(name); - return params.priority; -} - static BOOL get_pulse_name_by_guid(const GUID *guid, char pulse_name[MAX_PULSE_NAME_LEN], EDataFlow *flow) { struct device_cache *device; diff --git a/dlls/winepulse.drv/winepulse.drv.spec b/dlls/winepulse.drv/winepulse.drv.spec index 7aeb1756ed1..b822fd08353 100644 --- a/dlls/winepulse.drv/winepulse.drv.spec +++ b/dlls/winepulse.drv/winepulse.drv.spec @@ -1,5 +1,4 @@ # MMDevAPI driver functions -@ stdcall -private GetPriority() AUDDRV_GetPriority @ stdcall -private GetEndpointIDs(long ptr ptr ptr ptr) AUDDRV_GetEndpointIDs @ stdcall -private GetAudioEndpoint(ptr ptr ptr) AUDDRV_GetAudioEndpoint @ stdcall -private GetAudioSessionManager(ptr ptr) AUDDRV_GetAudioSessionManager -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1543
Davide Beatrici (@davidebeatrici) commented about dlls/ntdll/unix/loader.c:
*/ NTSTATUS WINAPI __wine_unix_call( unixlib_handle_t handle, unsigned int code, void *args ) { - return ((unixlib_entry_t*)(UINT_PTR)handle)[code]( args ); + unixlib_entry_t *funcs = (unixlib_entry_t*)(UINT_PTR)handle; + if (funcs[code]) + return funcs[code](args); How should the code style be here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_17402
Rémi Bernon (@rbernon) commented about dlls/ntdll/unix/loader.c:
*/ NTSTATUS WINAPI __wine_unix_call( unixlib_handle_t handle, unsigned int code, void *args ) { - return ((unixlib_entry_t*)(UINT_PTR)handle)[code]( args ); + unixlib_entry_t *funcs = (unixlib_entry_t*)(UINT_PTR)handle; + if (funcs[code]) Imho it would be better to not have NULL entry point in the first place, and return that status from them as needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_17424
On Fri Nov 25 08:37:46 2022 +0000, Rémi Bernon wrote:
Imho it would be better to not have NULL entry point in the first place, and return that status from them as needed. Sounds good to me, we can define a single dummy function that returns `STATUS_NOT_IMPLEMENTED` and set it for all fields that are currently `NULL`.
Should I prepend a commit for that? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_17426
On Fri Nov 25 08:48:45 2022 +0000, Davide Beatrici wrote:
Sounds good to me, we can define a single dummy function that returns `STATUS_NOT_IMPLEMENTED` and set it for all fields that are currently `NULL`. Should I prepend a commit for that? Alternatively, we can return `STATUS_SUCCESS` and do nothing else (aside from setting `Priority_Neutral` for `test_connect`).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_17529
On Fri Feb 24 06:40:42 2023 +0000, Olivia/Isabelle Mahadevan wrote:
Update? @huw Ping.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_25365
On Fri Feb 24 06:40:42 2023 +0000, Davide Beatrici wrote:
@huw Ping. Could you rebase this please? Also, I agree with @rbernon, it would be nicer not to have NULL entry points.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1543#note_25709
participants (5)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)
-
Olivia/Isabelle Mahadevan (＠doubledge-spec)
-
Rémi Bernon