Skip to content

Policy on tolerance of invalid parameters: programming error or API guarantee? #9235

@smcv

Description

@smcv

Most public API functions in SDL that are documented to take a SDL_Thing * parameter check for NULL (which is clearly not a valid pointer to a SDL_Thing *), and return some sort of error (usually NULL or 0 or -1 with the error indicator set) if the parameter was invalid. Similarly some non-pointer parameters are technically able to be set to out-of-range values, like a surface width that must be non-negative. The consequence is that programs end up accidentally (or maybe even intentionally) relying on invalid values not causing a crash, like in https://bugs.debian.org/1065714, which makes fixes like #9234 critical-path for making those games not regress.

Is it considered to be an API guarantee that these invalid parameters report an error without crashing, similar to the way Linux syscalls like access(NULL, F_OK) can be relied on to fail with EFAULT rather than crashing, and access("/", 9999) can be relied on to fail with EINVAL? Or is it considered to be undefined behaviour, similar to g_object_unref(NULL) in GLib, and SDL is just being helpful?

If invalid parameters are considered to be UB that should never be reached by a valid program, then it might be a good idea to SDL_LogWarn or SDL_LogError when they happen (in addition to setting the error indicator), so that there's a reminder to callers to fix their code. This is the approach taken in GLib and libdbus (both use the term "checks" for their parameter validation): they warn on stderr and attempt to continue, but there's an environment variable you can set to turn these warnings into a hard crash so that you can get a backtrace. They also have compile-time options intended for performance-critical scenarios with tight control over the code that is being run, which speed up the library by deleting the "checks" and blindly assuming that all parameters are valid, similar to the typical behaviour of libc functions like strcmp(), which will typically lead to crashes with a NULL pointer dereference if the library is used incorrectly. This would be a high regression risk for the copies of SDL2 in places like Linux distributions and the Steam Runtime, but it might be OK to have this as a policy change in SDL3, or it might be OK for a bundled application-specific SDL like the SDL3 snapshots used by Steam and CS2 (if the application developers are sufficiently careful).

Conversely, if it's considered to be an API guarantee that these checks are done, then it would probably be good to have systematic test coverage that calls each SDL function with NULL parameters to prove that it can be done.

Some APIs, like the SDL_Joystick API, also make a best-effort attempt to check the object for a "magic number" to detect use-after-free and similar situations, analogous to GLib's g_return_if_fail (G_IS_OBJECT (object)). This can obviously never be completely reliable, because if the pointer is dangling/freed/invalid, then any attempt to dereference it to look at the magic number is undefined behaviour - so I think the only option for this case is to say that it's UB, and SDL is just doing its best to be helpful but cannot make an API guarantee that invalid parameters won't crash it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions