obsproject
UI: Set invalid parameter handler in non-debug builds
Description
Sets the invalid parameter handler for Windows's "secure" functions like strcat_s(). By having one that does not abort execution we let the caller handle the error codes, instead of crashing to desktop.
Motivation and Context
We have error handling, we don't need Windows to abort execution of the program.
How Has This Been Tested?
Tested with issues found with #10834
Types of changes
- Tweak (non-breaking change to improve existing functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
This seems to go against Microsoft's recommendations:
When the framework calls the invalid parameter function, it usually means that a nonrecoverable error occurred. The invalid parameter function should save any data it can and then abort. It should not return control to the main function unless you are confident that the error is recoverable.
Specifically for strcat_s, it seems this would be called only when there is a programming error:
If strDestination is a null pointer, or isn't null-terminated, or if strSource is a NULL pointer, or if the destination string is too small, the invalid parameter handler is invoked, as described in Parameter validation.
In which case, I think I would prefer it invoke the crash handler. If we don't want this parameter validation we can use other non-throwing string functions such as StringCbCatA.
Specifically for
strcat_s, it seems this would be called only when there is a programming error:
In this case there is one, see #10837, which results in a non-terminated string.
However, strcat_s will return EINVAL in this case, which could be gracefully handled. We generally treat invalid input into our string conversion functions as non-fatal if we can detect it and simply return a status that indicates the data is invalid. This behaviour of the Microsoft implemenation to call a separate function is different from the C11 standard, which merely requires a non-zero return code. It seems weird for this specific case to cause a crash when it is decidedly not unrecoverable.
In which case, I think I would prefer it invoke the crash handler. If we don't want this parameter validation we can use other non-throwing string functions such as
StringCbCatA.
I haven't tried the crash handler yet, I wonder if it would give us anything useful in non-debug builds where all the descriptiors are NULL?