RegisterClass*(): Fix a crash if lpszMenuName is an integer resource ID.

R~ight, MAKEINTRESOURCE(). In a higher-level language, these resource IDs
(which can either be string pointers or 16-bit integers) would have gotten a
completely different type, and here, they don't even get their own Hungarian
notation prefix.

Creating a new union type to have perfect type safety would have been very
nice, but that's not really practical either. Therefore, a typedef, some
generic macros, and more thorough testing is really the only thing we can do
to keep this sort of thing from happening again.

src/macros.h

@@ -29,6 +29,34 @@
 #include <malloc.h>
 #endif
 
+/**
+  * Resource identifier type. Either a multi-byte or wide string or, if the
+  * high word is 0, an integer. Therefore, pretending that these are just
+  * mere strings (as both their variable name *and* their Hungarian notation
+  * prefix would suggest) is not only misleading, but downright harmful.
+  *
+  * I thought about using the completely type-safe approach of defining a
+  * union for both string types. This would throw a compile error when a RESID
+  * is accessed as if it were a string. However, there are pretty much only
+  * drawbacks to that approach:
+  * • This would exactly require to redeclare *all* structures to use the
+  *   (W)RESID unions where applicable.
+  * • Pretty much all of the A and W structures are typedef'd and thus can't
+  *   easily be re-#define'd like the functions. This means that, in the case
+  *   of static linking, these new declarations then wouldn't even propagate
+  *   to existing application code.
+  * • It would break compilation of existing, perfectly fine code as all
+  *   function calls that pass a RESID union in one of their parameters or
+  *   structures would also throw a compiler error.
+  * • Finally, even if we only used the RESID union internally ourselves, it
+  *   only adds complication to the conversion macros, which would need to
+  *   validly convert the input "strings" into RESID unions.
+  * So, it's just down to a mere typedef alias.
+  */
+
+typedef const char* RESID;
+typedef const wchar_t* WRESID;
+
 // Most Win32 API functions return TRUE on success and FALSE on failure,
 // requiring a separate call to GetLastError() to get the actual error code.
 // This macro wraps these function calls to use the opposite, more sensible
@@ -112,9 +140,29 @@
 #endif
 
 // Convenience macro to convert one fixed-length string to UTF-16.
-// TODO: place this somewhere else?
 #define FixedLengthStringConvert(str_in, str_len) \
 	size_t str_in##_len = (str_len != -1 ? str_len : strlen(str_in)) + 1; \
 	VLA(wchar_t, str_in##_w, str_in##_len); \
 	ZeroMemory(str_in##_w, str_in##_len * sizeof(wchar_t)); \
 	StringToUTF16(str_in##_w, str_in, str_len);
+
+// Now, if Microsoft just had used integer identifiers for resources instead
+// of names plus the MAKEINTRESOURCE hack, we could just re-point all these
+// calls to their wide versions and be done with it.
+// Instead, there is some maintenance to do...
+#define RESID_DEC(local) \
+	LPWSTR local##_w = NULL;
+
+#define RESID_CONV(local, src) \
+	if(HIWORD(src) != 0) { \
+		size_t local##_len = strlen(src) + 1; \
+		VLA(wchar_t, local##_w_vla, local##_len); \
+		local##_w = StringToUTF16_VLA(local##_w_vla, src, local##_len); \
+	} else { \
+		local##_w = (LPWSTR)(src); \
+	}
+
+#define RESID_FREE(local, src) \
+	if(HIWORD(src) != 0) { \
+		WCHAR_T_FREE(local); \
+	}

src/user32_dll.c

@@ -56,18 +56,16 @@ const w32u8_pair_t user32_pairs[] = {
 
 #define WndclassAToW(w, a) \
 	size_t lpszClassName_len = strlen((a)->lpszClassName) + 1; \
-	size_t lpszMenuName_len = strlen((a)->lpszMenuName) + 1; \
 	VLA(wchar_t, lpszClassName_w, lpszClassName_len); \
-	VLA(wchar_t, lpszMenuName_w, lpszMenuName_len); \
-	lpszClassName_w = StringToUTF16_VLA(lpszClassName_w, (a)->lpszClassName, lpszClassName_len); \
-	lpszMenuName_w = StringToUTF16_VLA(lpszMenuName_w, (a)->lpszMenuName, lpszMenuName_len); \
-	(w)->lpszClassName = lpszClassName_w; \
+	RESID_DEC(lpszMenuName); \
+	(w)->lpszClassName = StringToUTF16_VLA(lpszClassName_w, (a)->lpszClassName, lpszClassName_len); \
+	RESID_CONV(lpszMenuName, (a)->lpszMenuName); \
 	(w)->lpszMenuName = lpszMenuName_w; \
 	WndclassCopyBase((w), (a))
 
-#define WndclassWClean() \
+#define WndclassWClean(a) \
 	VLA_FREE(lpszClassName_w); \
-	VLA_FREE(lpszMenuName_w)
+	RESID_FREE(lpszMenuName, (a)->lpszMenuName);
 /// ---------------------
 
 LPSTR WINAPI CharNextU(
@@ -100,36 +98,21 @@ LPSTR WINAPI CharNextU(
 	return ret;
 }
 
-// Now, if Microsoft just had used integer identifiers for resources instead
-// of names plus the MAKEINTRESOURCE hack, we could just re-point
-// all these calls to their wide versions and be done with it.
-// Instead, there is some maintenance to do...
-#define ResourceBaseConvert(lpTemplateName) \
-	LPWSTR lptn_w = NULL; \
-	if(HIWORD(lpTemplateName) != 0) { \
-		WCHAR_T_DEC(lpTemplateName); \
-		WCHAR_T_CONV_VLA(lpTemplateName); \
-	} else { \
-		lptn_w = (LPWSTR)lpTemplateName; \
-	}
-
-#define ResourceBaseClean(lpTemplateName) \
-	if(HIWORD(lpTemplateName) != 0) { \
-		VLA_FREE(lptn_w); \
-	}
-
 HWND WINAPI CreateDialogParamU(
 	__in_opt HINSTANCE hInstance,
-	__in LPCSTR lpTemplateName,
+	__in RESID lpTemplateRes,
 	__in_opt HWND hWndParent,
 	__in_opt DLGPROC lpDialogFunc,
 	__in LPARAM dwInitParam
 )
 {
 	HWND ret;
-	ResourceBaseConvert(lpTemplateName);
-	ret = CreateDialogParamW(hInstance, lptn_w, hWndParent, lpDialogFunc, dwInitParam);
-	ResourceBaseClean(lpTemplateName);
+	RESID_DEC(lpTemplateRes);
+	RESID_CONV(lpTemplateRes, lpTemplateRes);
+	ret = CreateDialogParamW(
+		hInstance, lpTemplateRes_w, hWndParent, lpDialogFunc, dwInitParam
+	);
+	RESID_FREE(lpTemplateRes, lpTemplateRes);
 	return ret;
 }
 
@@ -165,16 +148,19 @@ HWND WINAPI CreateWindowExU(
 
 INT_PTR WINAPI DialogBoxParamU(
 	__in_opt HINSTANCE hInstance,
-	__in LPCSTR lpTemplateName,
+	__in RESID lpTemplateRes,
 	__in_opt HWND hWndParent,
 	__in_opt DLGPROC lpDialogFunc,
 	__in LPARAM dwInitParam
 )
 {
 	INT_PTR ret;
-	ResourceBaseConvert(lpTemplateName);
-	ret = DialogBoxParamW(hInstance, lptn_w, hWndParent, lpDialogFunc, dwInitParam);
-	ResourceBaseClean(lpTemplateName);
+	RESID_DEC(lpTemplateRes);
+	RESID_CONV(lpTemplateRes, lpTemplateRes);
+	ret = DialogBoxParamW(
+		hInstance, lpTemplateRes_w, hWndParent, lpDialogFunc, dwInitParam
+	);
+	RESID_FREE(lpTemplateRes, lpTemplateRes);
 	return ret;
 }
 
@@ -277,7 +263,7 @@ ATOM WINAPI RegisterClassU(
 	WNDCLASSW WndClassW;
 	WndclassAToW(&WndClassW, lpWndClass);
 	ret = RegisterClassW(&WndClassW);
-	WndclassWClean();
+	WndclassWClean(lpWndClass);
 	return ret;
 }
 
@@ -290,7 +276,7 @@ ATOM WINAPI RegisterClassExU(
 	WndclassAToW(&WndClassW, lpWndClass);
 	WndclassExCopyBase(&WndClassW, lpWndClass);
 	ret = RegisterClassExW(&WndClassW);
-	WndclassWClean();
+	WndclassWClean(lpWndClass);
 	return ret;
 }
 

src/user32_dll.h

@@ -16,7 +16,7 @@ LPSTR WINAPI CharNextU(
 
 HWND WINAPI CreateDialogParamU(
 	__in_opt HINSTANCE hInstance,
-	__in LPCSTR lpTemplateName,
+	__in RESID lpTemplateRes,
 	__in_opt HWND hWndParent,
 	__in_opt DLGPROC lpDialogFunc,
 	__in LPARAM dwInitParam
@@ -47,7 +47,7 @@ HWND WINAPI CreateWindowExU(
 
 INT_PTR WINAPI DialogBoxParamU(
 	__in_opt HINSTANCE hInstance,
-	__in LPCSTR lpTemplateName,
+	__in RESID lpTemplateRes,
 	__in_opt HWND hWndParent,
 	__in_opt DLGPROC lpDialogFunc,
 	__in LPARAM dwInitParam