We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3321 - SDL_Init(SDL_INIT_VIDEO) always sets error on Win 7 and below
Summary: SDL_Init(SDL_INIT_VIDEO) always sets error on Win 7 and below
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.4
Hardware: x86_64 Windows 7
: P2 minor
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-05 22:25 UTC by Jason Francis
Modified: 2017-08-12 03:19 UTC (History)
1 user (show)

See Also:

Attachments
Remove SDL_LoadObject and SDL_LoadFunction calls in WIN_CreateDevice (4.73 KB, patch)
2016-05-05 22:25 UTC, Jason Francis 		Details | Diff
Test case (444 bytes, text/x-csrc)
2016-05-05 22:33 UTC, Jason Francis 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Francis 2016-05-05 22:25:47 UTC 
Created attachment 2446 [details]
Remove SDL_LoadObject and SDL_LoadFunction calls in WIN_CreateDevice

Calling SDL_GetError() after SDL_Init(SDL_INIT_VIDEO) on Windows 7 or earlier will always return an error about a missing SHCORE.DLL. The error is coming from use of SDL_LoadObject, which sets an error when it fails to load a file. The SHCORE.DLL file only exists on Win8 or above,.

The WIN_CreateDevice function in src/video/windows/SDL_windowsvideo.c tries to dynamically load SHCORE.DLL in order to use the "GetDpiForMonitor" function. This function additionally only exists on Win8.1 or above, as per the MSDN documentation: https://msdn.microsoft.com/en-us/library/windows/desktop/dn280510(v=vs.85).aspx

The use of GetDpiForMonitor in SDL is optional and only comes as an added bonus for when the user is on a system that has it available. For this reason I think it is not appropriate to use SDL_LoadObject or SDL_LoadFunction in order to access optional DLLs. I've attached a patch that fixes this issue by just using LoadLibrary and GetProcAddress in the WIN_CreateDevice function. This retains the optional functionality but prevents SDL_Init from setting an error when an optional DLL does not exist.
Comment 1 Jason Francis 2016-05-05 22:33:23 UTC 
Created attachment 2447 [details]
Test case

Attached a test case. This will always print an error about SHCORE.DLL missing on Win7, but will print nothing on Win8.1 or Win10. I don't have access to a Win8 machine to test.
Comment 2 Ozkan Sezer 2016-05-24 08:00:05 UTC 
A much simpler solution would be clearing the error, e.g.:

diff -r 7cbfd97f1430 src/video/windows/SDL_windowsvideo.c
--- a/src/video/windows/SDL_windowsvideo.c	Mon May 23 15:29:25 2016 -0300
+++ b/src/video/windows/SDL_windowsvideo.c	Tue May 24 10:55:10 2016 +0300
@@ -119,6 +119,9 @@ WIN_CreateDevice(int devindex)
     if (data->shcoreDLL) {
         data->GetDpiForMonitor = (HRESULT (WINAPI *)(HMONITOR, MONITOR_DPI_TYPE, UINT *, UINT *)) SDL_LoadFunction(data->shcoreDLL, "GetDpiForMonitor");
     }
+    else {
+        SDL_ClearError();
+    }
 
     /* Set the function pointers */
     device->VideoInit = WIN_VideoInit;
Comment 3 Alex Szpakowski 2016-05-24 16:47:51 UTC 
Note that SDL_GetError should *only* be used when a SDL function returns indicating an error (-1 or NULL, usually). SDL_GetError should not be used for checking whether an error has occurred at all - it may return anything it wants.
Comment 4 Sam Lantinga 2017-08-12 03:19:31 UTC 
You should only check the SDL error if an API function fails. That said, this was cleaned up in this commit:
https://hg.libsdl.org/SDL/rev/f94279990934