main: Update GDK argv handling to match new Windows implementation

Author: castholm

src/main/gdk/SDL_sysmain_runapp.cpp

@@ -31,58 +31,76 @@ extern "C" {
 #include <shellapi.h> // CommandLineToArgvW()
 #include <appnotify.h>
 
-// Pop up an out of memory message, returns to Windows
-static BOOL OutOfMemory(void)
+static int OutOfMemory(void)
 {
     SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Fatal Error", "Out of memory - aborting", NULL);
-    return FALSE;
+    return -1;
+}
+
+static int ErrorProcessingCommandLine(void)
+{
+    SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Fatal Error", "Error processing command line arguments", NULL);
+    return -1;
 }
 
-/* Gets the arguments with GetCommandLine, converts them to argc and argv
-   and calls SDL_main */
 extern "C"
-int SDL_RunApp(int, char**, SDL_main_func mainFunction, void *)
+int SDL_RunApp(int caller_argc, char* caller_argv[], SDL_main_func mainFunction, void *)
 {
-    LPWSTR *argvw;
-    char **argv;
-    int i, argc, result;
+    int result, argc;
+    LPWSTR *argvw = NULL;
+    char **argv = NULL;
     HRESULT hr;
     XTaskQueueHandle taskQueue;
 
-    argvw = CommandLineToArgvW(GetCommandLineW(), &argc);
-    if (argvw == NULL) {
-        return OutOfMemory();
-    }
+    // Note that we need to be careful about how we allocate/free memory in this function. If the application calls
+    // SDL_SetMemoryFunctions(), we can't rely on SDL_free() to use the same allocator after SDL_main() returns.
 
-    /* Note that we need to be careful about how we allocate/free memory here.
-     * If the application calls SDL_SetMemoryFunctions(), we can't rely on
-     * SDL_free() to use the same allocator after SDL_main() returns.
-     */
+    if (!caller_argv || caller_argc < 0) {
+        // If the passed argv is NULL or argc is negative, the user expects SDL to get the command line arguments
+        // using GetCommandLineW() and convert them to argc and argv before calling mainFunction().
 
-    // Parse it into argv and argc
-    argv = (char **)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, (argc + 1) * sizeof(*argv));
-    if (argv == NULL) {
-        return OutOfMemory();
-    }
-    for (i = 0; i < argc; ++i) {
-        const int utf8size = WideCharToMultiByte(CP_UTF8, 0, argvw[i], -1, NULL, 0, NULL, NULL);
-        if (!utf8size) {  // uhoh?
-            SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Fatal Error", "Error processing command line arguments", NULL);
-            return -1;
+        // Because of how the Windows command line works, we know for sure that the buffer size required to store all
+        // argument strings converted to UTF-8 (with null terminators) is guaranteed to be less than or equal to the
+        // size of the original command line string converted to UTF-8.
+        const int argdata_size = WideCharToMultiByte(CP_UTF8, 0, GetCommandLineW(), -1, NULL, 0, NULL, NULL); // Includes the null terminator
+        if (!argdata_size) {
+            result = ErrorProcessingCommandLine();
+            goto cleanup;
         }
 
-        argv[i] = (char *)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, utf8size);  // this size includes the null-terminator character.
-        if (!argv[i]) {
-            return OutOfMemory();
+        argvw = CommandLineToArgvW(GetCommandLineW(), &argc);
+        if (!argvw || argc < 0) {
+            result = OutOfMemory();
+            goto cleanup;
         }
 
-        if (WideCharToMultiByte(CP_UTF8, 0, argvw[i], -1, argv[i], utf8size, NULL, NULL) == 0) {  // failed? uhoh!
-            SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Fatal Error", "Error processing command line arguments", NULL);
-            return -1;
+        // Allocate argv followed by the argument string buffer as one contiguous allocation.
+        argv = (char **)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, (argc + 1) * sizeof(*argv) + argdata_size);
+        if (!argv) {
+            result = OutOfMemory();
+            goto cleanup;
+        }
+        char *argdata = ((char *)argv) + (argc + 1) * sizeof(*argv);
+        int argdata_index = 0;
+
+        for (int i = 0; i < argc; ++i) {
+            const int bytes_written = WideCharToMultiByte(CP_UTF8, 0, argvw[i], -1, argdata + argdata_index, argdata_size - argdata_index, NULL, NULL);
+            if (!bytes_written) {
+                result = ErrorProcessingCommandLine();
+                goto cleanup;
+            }
+            argv[i] = argdata + argdata_index;
+            argdata_index += bytes_written;
         }
+        argv[argc] = NULL;
+
+        argvw = NULL;
+
+        caller_argc = argc;
+        caller_argv = argv;
     }
-    argv[i] = NULL;
-    LocalFree(argvw);
+
+    // !!! FIXME: This function does not currently properly deinitialize GDK resources on failure.
 
     hr = XGameRuntimeInitialize();
 
@@ -100,6 +118,7 @@ int SDL_RunApp(int, char**, SDL_main_func mainFunction, void *)
             xblArgs.queue = taskQueue;
             SDL_snprintf(scidBuffer, 64, "00000000-0000-0000-0000-0000%08X", titleid);
             xblArgs.scid = scidBuffer;
+            // !!! FIXME: XblInitialize() should have a corresponding call to XblCleanup() according to the docs.
             hr = XblInitialize(&xblArgs);
         } else {
             SDL_SetError("[GDK] Unable to get titleid. Will not call XblInitialize. Check MicrosoftGame.config!");
@@ -111,8 +130,7 @@ int SDL_RunApp(int, char**, SDL_main_func mainFunction, void *)
             return -1;
         }
 
-        // Run the application main() code
-        result = SDL_CallMain(argc, argv, mainFunction);
+        result = mainFunction(caller_argc, caller_argv); // No need for SDL_CallMain(); we already know that we have a valid argv
 
         GDK_UnregisterChangeNotifications();
 
@@ -138,12 +156,10 @@ int SDL_RunApp(int, char**, SDL_main_func mainFunction, void *)
         result = -1;
     }
 
-    // Free argv, to avoid memory leak
-    for (i = 0; i < argc; ++i) {
-        HeapFree(GetProcessHeap(), 0, argv[i]);
-    }
+cleanup:
+
     HeapFree(GetProcessHeap(), 0, argv);
+    LocalFree(argvw);
 
     return result;
 }
-