(Win32 refresh) GraphicsWindow

Bring in std::optional<DEVMODE> to gracefully handle the case where the function may not return anything valid. This helps to avoid undefined behavior caused by unexpected/invalid return values, as an extra protection to supplement the check if we can identify the refresh rate.

src/archutils/Win32/GraphicsWindow.cpp

@@ -14,6 +14,7 @@
 #include "CommandLineActions.h"
 #include "DirectXHelpers.h"
 
+#include <optional>
 #include <set>
 
 static const RString g_sClassName = PRODUCT_ID;
@@ -50,6 +51,19 @@ static RString GetNewWindow()
 	return sName;
 }
 
+static std::optional<DEVMODE> GetDisplaySettings(const RString& displayId)
+{
+	DEVMODE dm;
+	ZERO( dm );
+	dm.dmSize = sizeof(dm);
+	if (!EnumDisplaySettings(displayId, ENUM_CURRENT_SETTINGS, &dm))
+	{
+		LOG->Warn("%s", werr_ssprintf(GetLastError(), "EnumDisplaySettings failed").c_str());
+		return std::nullopt;
+	}
+	return dm;
+}
+
 static LRESULT CALLBACK GraphicsWindow_WndProc( HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam )
 {
 	CHECKPOINT_M( ssprintf("%p, %u, %08x, %08x", hWnd, msg, wParam, lParam) );
@@ -182,35 +196,26 @@ static LRESULT CALLBACK GraphicsWindow_WndProc( HWND hWnd, UINT msg, WPARAM wPar
 	CHECKPOINT_M( ssprintf("%p, %u, %08x, %08x", hWnd, msg, wParam, lParam) );
 
 	if( m_bWideWindowClass )
+	{
 		return DefWindowProcW( hWnd, msg, wParam, lParam );
+	}
 	else
+	{
 		return DefWindowProcA( hWnd, msg, wParam, lParam );
+	}
 }
 
 static void AdjustVideoModeParams( VideoModeParams &p )
 {
-	DEVMODE dm;
+	std::optional<DEVMODE> dmOpt = GetDisplaySettings(p.sDisplayId);
-	ZERO( dm );
+	if (!dmOpt.has_value())
-	dm.dmSize = sizeof(dm);
-	if (!EnumDisplaySettings(p.sDisplayId, ENUM_CURRENT_SETTINGS, &dm))
 	{
 		p.rate = 60;
-		LOG->Warn( "%s", werr_ssprintf(GetLastError(), "EnumDisplaySettings failed").c_str() );
 		return;
 	}
 
-	/* On a nForce 2 IGP on Windows 98, dm.dmDisplayFrequency sometimes 
+	const DEVMODE& dm = *dmOpt;
-	 * (but not always) is 0.
+	if (!(dm.dmFields & DM_DISPLAYFREQUENCY) || dm.dmDisplayFrequency == 0 || dm.dmDisplayFrequency == 1)
-	 *
-	 * MSDN: When you call the EnumDisplaySettings function, the 
-	 * dmDisplayFrequency member may return with the value 0 or 1. 
-	 * These values represent the display hardware's default refresh rate. 
-	 * This default rate is typically set by switches on a display card or 
-	 * computer motherboard, or by a configuration program that does not 
-	 * use Win32 display functions such as ChangeDisplaySettings. */
-	if( !(dm.dmFields & DM_DISPLAYFREQUENCY) ||
-		dm.dmDisplayFrequency == 0 ||
-		dm.dmDisplayFrequency == 1 )
 	{
 		p.rate = 60;
 		LOG->Warn( "EnumDisplaySettings doesn't know what the refresh rate is. %d %d %d", dm.dmPelsWidth, dm.dmPelsHeight, dm.dmBitsPerPel );
@@ -233,9 +238,13 @@ RString GraphicsWindow::SetScreenMode( const VideoModeParams &p )
 		return RString();
 	}
 
-	DEVMODE DevMode;
+	std::optional<DEVMODE> dmOpt = GetDisplaySettings(p.sDisplayId);
-	ZERO( DevMode );
+	if (!dmOpt.has_value())
-	DevMode.dmSize = sizeof(DEVMODE);
+	{
+		return "Couldn't retrieve display settings";
+	}
+
+	DEVMODE DevMode = *dmOpt;
 	DevMode.dmPelsWidth = p.width;
 	DevMode.dmPelsHeight = p.height;
 	DevMode.dmBitsPerPel = p.bpp;
@@ -255,9 +264,11 @@ RString GraphicsWindow::SetScreenMode( const VideoModeParams &p )
 		ret = ChangeDisplaySettingsEx( p.sDisplayId, &DevMode, nullptr, CDS_FULLSCREEN, nullptr );
 	}
 
-	// XXX: append error
 	if( ret != DISP_CHANGE_SUCCESSFUL )
+	{
+		LOG->Warn("ChangeDisplaySettingsEx failed with error code: %d", ret);
 		return "Couldn't set screen mode";
+	}
 
 	g_FullScreenDevMode = DevMode;
 	return RString();