| Summary: | use GetModuleHandleW() to retrieve kernel32.dll handle. | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Ozkan Sezer <sezeroz> |
| Component: | *don't know* | Assignee: | Ozkan Sezer <sezeroz> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | jl |
| Version: | HG 2.0 | Keywords: | target-2.0.16 |
| Hardware: | All | ||
| OS: | Windows (All) | ||
| Attachments: |
GetModuleHandle patch
GetModuleHandle patch WinRT patch Statically link synchronization APIs on WINRT Remove synchronization.lib from WinPhone81 Disable WaitOnAddress SDL_sem implementation on Windows Phone Disable WaitOnAddress on Windows Phone -- V2 |
||
Created attachment 4574 [details]
GetModuleHandle patch
(updated patch to fix commit message.)
Pushed the patch as https://hg.libsdl.org/SDL/rev/df2a7472308f Closing as fixed. This doesn't appear to work with the older Windows Phone SDK: 1>..\..\src\thread\windows\SDL_syssem.c(396): warning C4013: 'GetModuleHandleW' undefined; assuming extern returning int 1>..\..\src\thread\windows\SDL_syssem.c(396): warning C4047: 'initializing' : 'HMODULE' differs in levels of indirection from 'int' 1>..\..\src\thread\windows\SDL_sysmutex.c(295): warning C4013: 'GetModuleHandleW' undefined; assuming extern returning int 1>..\..\src\thread\windows\SDL_sysmutex.c(295): warning C4047: 'initializing' : 'HMODULE' differs in levels of indirection from 'int' Ozkan, can you look at this? I do not know anything about Windows Phone, but aren't the project files supposed to use winrt/* sources instead of windows/*.c ones? If they have to use windows/*.c sources, can we conditionalize the GetModuleHandleW usage using __WINRT__ ifdefs? Created attachment 4601 [details]
WinRT patch
OK, does the attached patch work correctly?
Added Joel Linn to the CC list: those are the files touched by him. Created attachment 4602 [details]
Statically link synchronization APIs on WINRT
I'm unable to find a VS2013 Community download but I think you are referring to the `SDL-WinPhone81` and `SDL-WinRT81` projects. They are special in that they do not use the `stdcpp` implementations like the `SDL-UWP` project does.
The way I understand the Windows header `#if`s, all the APIs we load dynamically should be available in `WINAPI_PARTITION_APP` which is part of `WINAPI_FAMILY_PHONE_APP`.
If possible I would just skip dynamic loading and link statically. I made a patch for that, can't test it for the WinPhone target personally though...
(In reply to Joel Linn from comment #8) > Created attachment 4602 [details] > Statically link synchronization APIs on WINRT > > I'm unable to find a VS2013 Community download but I think you are referring > to the `SDL-WinPhone81` and `SDL-WinRT81` projects. They are special in that > they do not use the `stdcpp` implementations like the `SDL-UWP` project does. > > The way I understand the Windows header `#if`s, all the APIs we load > dynamically should be available in `WINAPI_PARTITION_APP` which is part of > `WINAPI_FAMILY_PHONE_APP`. > > If possible I would just skip dynamic loading and link statically. I made a > patch for that, can't test it for the WinPhone target personally though... I applied this patch as https://hg.libsdl.org/SDL/rev/879390cc2733 Let's see how buildbot reacts to it. No warnings at all, but linkage fails with 1>LINK : fatal error LNK1181: cannot open input file 'synchronization.lib' https://buildbot.libsdl.org/#/builders/8/builds/408 (In reply to Ozkan Sezer from comment #10) > No warnings at all, but linkage fails with > 1>LINK : fatal error LNK1181: cannot open input file 'synchronization.lib' > > https://buildbot.libsdl.org/#/builders/8/builds/408 Oh wow. On Desktop `WaitOnAddress` is imported from that lib so I added it. Let me see, maybe I can dig up an old VS2013 and check with a phone project... Created attachment 4603 [details] Remove synchronization.lib from WinPhone81 I checked with VS2013. While linking to `synchronization.lib` is required (and correct) for Desktop and UWP apps (https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitonaddress), the required functions of the synch API seem to be implicitly available on Windows Phone 8.1 without setting additional dependencies. Thanks. Does WinRT81_VS2013 project not need the same? (In reply to Ozkan Sezer from comment #13) > Thanks. Does WinRT81_VS2013 project not need the same? No, it is correct (and required) to link against that library for Desktop and Desktop Store Apps. Created attachment 4604 [details] Disable WaitOnAddress SDL_sem implementation on Windows Phone Although it links on WinPhone now, the APIs are not actually legal to use :/ https://www.microsoft.com/en-us/download/details.aspx?id=47328 Output from appcert.exe for a test APP that links to the APIs in question: ``` <REQUIREMENT NUMBER="15" TITLE="Supported API test" RATIONALE="The application should only refer to the APIs allowed by the Windows SDK for Windows Store Apps."> <TEST INDEX="72" NAME="Supported APIs" DESCRIPTION="Windows Phone App must only use supported platform APIs." EXECUTIONTIME="00h:00m:01s.11ms" OPTIONAL="FALSE"> <MESSAGES> <MESSAGE TEXT="This API is not supported for this application type - Api=WaitOnAddress. Module=api-ms-win-core-synch-l1-2-0.dll. File=App1.exe." /> <MESSAGE TEXT="This API is not supported for this application type - Api=WakeByAddressSingle. Module=api-ms-win-core-synch-l1-2-0.dll. File=App1.exe." /> </MESSAGES> <RESULT><![CDATA[FAIL]]></RESULT> </TEST> </REQUIREMENT> ``` Created attachment 4607 [details] Disable WaitOnAddress on Windows Phone -- V2 (In reply to Joel Linn from comment #15) > Created attachment 4604 [details] > Disable WaitOnAddress SDL_sem implementation on Windows Phone > > Although it links on WinPhone now, the APIs are not actually legal to use :/ > https://www.microsoft.com/en-us/download/details.aspx?id=47328 Modified your patch for compatibility: not all SDKs have WINAPI_FAMILY_PHONE_APP (or even have winapifamily.h for that matter.) Please confirm / review. (In reply to Ozkan Sezer from comment #16) > Created attachment 4607 [details] > Disable WaitOnAddress on Windows Phone -- V2 > > (In reply to Joel Linn from comment #15) > > Created attachment 4604 [details] > > Disable WaitOnAddress SDL_sem implementation on Windows Phone > > > > Although it links on WinPhone now, the APIs are not actually legal to use :/ > > https://www.microsoft.com/en-us/download/details.aspx?id=47328 > > Modified your patch for compatibility: not all SDKs have > WINAPI_FAMILY_PHONE_APP (or even have winapifamily.h for > that matter.) Please confirm / review. Builds fine for VS2013 WinRT81/WinPhone81 and VS2019 CMake OK then, should be fixed by https://hg.libsdl.org/SDL/rev/b69400b305a5 Closing. |
Created attachment 4573 [details] GetModuleHandle patch SDL_systhread.c and SDL_syslocale.c used to call LoadLibrary() without calling FreeLibrary() later. GetModuleHandleW() should always succeed because GetModuleHandleW() itself is imported from kernel32.dll and we don't need to bother releasing it.