Skip to content
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

Filesystem [2.75/4]: Add sanity checks after OpenInput/OutputStream #2471

merged 2 commits into from Mar 10, 2021


Copy link

@Ghabry Ghabry commented Mar 7, 2021

OpenInput/OutputStream are invalid when the stream cannot be opened.
This results in segfaults when they are passed by reference.

Most important is a regression fix: When RPG_RT.ini was missing the Player crashed :(

For all others: Good to have (most of them are unlikely to be hit because other code checks first if a file was found so they will only fail for stuff like "No permission" or "Out of handles")

@Ghabry Ghabry added this to the 0.6.3 milestone Mar 7, 2021
@@ -145,7 +150,7 @@ bool Input::Source::InitRecording(const std::string& record_to_path) {
record_log = std::make_unique<Filesystem_Stream::OutputStream>(FileFinder::OpenOutputStream(path, std::ios::out | std::ios::trunc));

if (!record_log) {
Output::Warning("Failed to open file {} for input recording : {}", path, strerror(errno));
Output::Error("Failed to open file {} for input recording : {}", path, strerror(errno));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

promoted this to an error because this is an incorrect command line argument

…reference in the error-case crashes.

Fixes a regression: Loading a game that had no RPG_RT.ini file crashed.
@fmatthew5876 fmatthew5876 merged commit cf8d55c into EasyRPG:master Mar 10, 2021
Copy link
Member Author

Ghabry commented Mar 10, 2021

Meh already found a problem:
When there is no easyrpg.soundfont it shows a warning on startup because the name is passed to vio_open.

Though the problem here isn't the warning but that there is no Exists-checks before as we do for Wildmidi. see #2300

@Ghabry Ghabry deleted the fs-fixes branch May 27, 2021 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Successfully merging this pull request may close these issues.

None yet

3 participants