-
Notifications
You must be signed in to change notification settings - Fork 2.1k
main: Change how SDL_RunApp()
handles and overrides the provided argv
#12676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
If the specified argv is NULL, `SDL_RunApp()` will now use a dummy argv instead, on all platforms.
This new implementation now only parses the command line and overrides the provided argv if the provided one is NULL. This enables programs that don't want to or can't use `SDL_main.h` to perform their own custom argument processing before explicitly calling `SDL_RunApp()`. If the program includes `SDL_main.h` as normal, the behavior remains the same as before (because `SDL_main_impl.h` passes a NULL argv). In addition, this new implementation performs fewer allocations and no longer leaks on failure.
@@ -114,7 +117,7 @@ | |||
* | |||
* \since This macro is available since SDL 3.2.0. | |||
*/ | |||
#define SDL_MAIN_AVAILABLE | |||
#define SDL_MAIN_AVAILABLE 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, I changed these because I think the wiki does not currently pick up defines that don't have any values. For example, https://wiki.libsdl.org/SDL3/SDL_MAIN_AVAILABLE doesn't exist.
/* Win32-specific SDL_RunApp(), which does most of the SDL_main work, | ||
based on SDL_windows_main.c, placed in the public domain by Sam Lantinga 4/13/98 */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this comment as to not misrepresent the origins of the code since I changed the implementation pretty drastically, but I could restore this if you prefer.
// 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://learn.microsoft.com/en-us/cpp/cpp/main-function-command-line-args?view=msvc-170#parsing-c-command-line-arguments for details.
The gist is that the arguments in the command-line string are always either bare
, "quoted"
or "escape \\"quotes\\" using backslashes"
, which means that when parsed, each argument always results in a string of the same length or shorter than the argument in the command-line string, never longer. In addition, arguments are always separated by at least one space character (and the entire command-line string is null-terminated), so if we use the length of the whole command-line string as the buffer size there's still enough room for null terminators even in the worst case (app arg1 arg2\0
-> app\0arg1\0arg2\0
)
Would it be possible to also pass the original args on other platforms when you pass NULL, like Windows? This would make it consistent across platforms and would also make my own optional wrapper around it simpler :v (arguments in Rust aren't null-terminated so I have to allocate new ones that are properly terminated for C. currently my wrapper doesn't pass through args at all but i should fix that either way) |
I could be wrong but AFAIK there's not really any convenient or reliable way of getting the argv outside of the native entry point on most platforms. Windows is actually kinda unique in that is has an easily accessible On Linux I've read that you can theoretically get the arguments by reading Needing to copy and null-terminate your Rust strings seems slightly annoying, but in the grand scheme of things your app will probably end up doing similar numbers of allocations if you copy it yourself as it would if SDL copied it from somewhere else. |
Motivation
I use SDL from a different language than C, which means that I can't use the macro magic in
SDL_main.h
and instead have to callSDL_RunApp()
manually from the native entry point to get SDL to perform all the necessary platform-specific setup. This language also has its own preferred way of obtaining the command-line arguments as an argv, which means that I need to get them and pass them forward toSDL_RunApp()
. On most platforms, this is fine and SDL will pass the provided argv along unmodified to the main function. However, on Windows, SDL ignores the provided argv completely and instead gets and parses the command-line string into a new argv and uses that instead.This not only means that my native entry point and
SDL_RunApp()
are both doing the same exact work twice, but it also becomes a problem if the original argv that was passed toSDL_RunApp()
did not actually come from the command-line (maybe I use a hard-coded argv for debugging/testing), or if it has been modified beforehand (maybe SDL is only used for part of my program and my intent is for a command-line invocation likeapp --backend=SDL arg1 arg2
to get passed on to the SDL-specific parts asapp arg1 arg2
), since SDL will overwrite the user-provided argv on Windows.I think it would be better and enable more advanced use cases (especially from non-C languages) if
SDL_RunApp()
only overrode the argv if the user didn't provide one (i.e. passed null).The changes
This PR changes the Windows and GDK implementations of
SDL_RunApp()
so that they properly respect the user-provided argv and forward it to the main function, and only fall back to parsing the Windows command-line string if theargv
parameter is null. Important to note is that the Windows/GDK entry points inSDL_main_impl.h
already always pass null toSDL_RunApp()
, so this change is backward-compatible and does not affect any apps that useSDL_main.h
.The only kind of usage this change is backward-incompatible with is if the user explicitly calls
SDL_RunApp()
with a non-null argv (e.g. an empty argv) and still expects SDL to override it. But the likelihood of there being some existing application that relies on this very specific behavior is not only extremely slim, the behavior also only occurs on Windows and has never been documented as guaranteed by the API, so I personally think it is safe to make this change. (If you're extra charitable you could also consider this a bug/spec fix that defines the behavior of some previously undefined edge cases in the API.)I also rewrote the way Windows/GDK processes the command-line to require fewer allocations, down from
2 + argc
to only 2, and ensured the allocations are properly freed even on failure. You could further reduce it down to just 1 if you parsed the command-line string on your own instead of usingCommandLineToArgvW()
, but that felt like too much work (especially considering it would probably require lots of unit tests to ensure the behavior is correct).Outside of the Windows-specific stuff, this PR also ensures that all other platforms use the
{ "SDL_app", NULL }
dummy argv as a fallback if the if the user passes null (previously only some platforms did this, which could result in null dereferences in user code).The doc comments in
SDL_main.h
have been updated to document the argv-handling behavior.Other stuff
This PR adds a new header file. I updated the Visual Studio project files but I don't have convenient access to Xcode at the moment so the Xcode project files might need to be updated by a maintainer.
I also don't have access to any of the NDA'd private implementations so I don't know if any of them need be updated to take any of the changes in this PR into consideration for maximum consistency between all platforms.