From 4b494a69bd47022ef75d51ab6fd5e080c087a903 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Sun, 9 Apr 2017 18:38:38 +0200 Subject: [PATCH] Rename fatalMessageBox() to fatalMessageBoxWithoutExit(), add exiting variant - Rename fatalMessageBox() to fatalMessageBoxWithoutExit(). This should prevent the kind of bug that prompted this set of changes: someone calling fatalMessageBox(), assuming the program would stop at that point, whereas in reality it did not. - Add new function fatalMessageBoxThenExit(). This is not vital of course, but allows one to spare one line here and there and to apply the DRY principle for such fatal exits. - Replace every existing call to fatalMessageBox() with one or the other of the two new functions. Improve formatting along the way. This fixes a few bugs of the kind explained above. --- src/GUI/MessageBox.cxx | 12 ++++- src/GUI/MessageBox.hxx | 11 ++++- src/Main/bootstrap.cxx | 13 ++++-- src/Main/fg_init.cxx | 88 +++++++++++++++++++++--------------- src/Main/options.cxx | 44 ++++++++++-------- src/Main/util.cxx | 25 +++++----- src/Navaids/NavDataCache.cxx | 14 +++--- tests/testStubs.cxx | 8 ++++ 8 files changed, 133 insertions(+), 82 deletions(-) diff --git a/src/GUI/MessageBox.cxx b/src/GUI/MessageBox.cxx index 0a36688a6..01cc07401 100644 --- a/src/GUI/MessageBox.cxx +++ b/src/GUI/MessageBox.cxx @@ -148,7 +148,7 @@ MessageBoxResult modalMessageBox(const std::string& caption, #endif } -MessageBoxResult fatalMessageBox(const std::string& caption, +MessageBoxResult fatalMessageBoxWithoutExit(const std::string& caption, const std::string& msg, const std::string& moreText) { @@ -175,4 +175,14 @@ MessageBoxResult fatalMessageBox(const std::string& caption, #endif } +[[noreturn]] void fatalMessageBoxThenExit( + const std::string& caption, + const std::string& msg, + const std::string& moreText, + int exitStatus) +{ + fatalMessageBoxWithoutExit(caption, msg, moreText); + exit(exitStatus); +} + } // of namespace flightgear diff --git a/src/GUI/MessageBox.hxx b/src/GUI/MessageBox.hxx index 686b53b78..ab4d62e05 100644 --- a/src/GUI/MessageBox.hxx +++ b/src/GUI/MessageBox.hxx @@ -2,6 +2,7 @@ #define FG_GUI_MESSAGE_BOX_HXX #include +#include namespace flightgear { @@ -17,10 +18,18 @@ MessageBoxResult modalMessageBox(const std::string& caption, const std::string& msg, const std::string& moreText = std::string()); -MessageBoxResult fatalMessageBox(const std::string& caption, +MessageBoxResult fatalMessageBoxWithoutExit( + const std::string& caption, const std::string& msg, const std::string& moreText = std::string()); +[[noreturn]] void fatalMessageBoxThenExit( + const std::string& caption, + const std::string& msg, + const std::string& moreText = std::string(), + int exitStatus = EXIT_FAILURE); + + } // of namespace flightgear #endif // of FG_GUI_MESSAGE_BOX_HXX diff --git a/src/Main/bootstrap.cxx b/src/Main/bootstrap.cxx index 5da593452..63a2b58cc 100644 --- a/src/Main/bootstrap.cxx +++ b/src/Main/bootstrap.cxx @@ -223,8 +223,10 @@ int main ( int argc, char **argv ) { #ifdef ENABLE_SIMD if (!detectSIMD()) { - flightgear::fatalMessageBox("Fatal error","SSE2 support not detetcted but this version of FlightGear requires SSE2 hardware support."); - return -1; + flightgear::fatalMessageBoxThenExit( + "Fatal error", + "SSE2 support not detected, but this version of FlightGear requires " + "SSE2 hardware support."); } #endif @@ -330,12 +332,13 @@ int main ( int argc, char **argv ) std::string info; if (std::strlen(t.getOrigin()) != 0) info = std::string("received from ") + t.getOrigin(); - flightgear::fatalMessageBox("Fatal exception", t.getFormattedMessage(), info); + flightgear::fatalMessageBoxWithoutExit( + "Fatal exception", t.getFormattedMessage(), info); } catch (const std::exception &e ) { - flightgear::fatalMessageBox("Fatal exception", e.what()); + flightgear::fatalMessageBoxWithoutExit("Fatal exception", e.what()); } catch (const std::string &s) { - flightgear::fatalMessageBox("Fatal exception", s); + flightgear::fatalMessageBoxWithoutExit("Fatal exception", s); } catch (const char *s) { std::cerr << "Fatal error (const char*): " << s << std::endl; diff --git a/src/Main/fg_init.cxx b/src/Main/fg_init.cxx index f2fd61458..edb464ef2 100644 --- a/src/Main/fg_init.cxx +++ b/src/Main/fg_init.cxx @@ -195,7 +195,8 @@ public: { std::string aircraft = fgGetString( "/sim/aircraft", ""); if (aircraft.empty()) { - flightgear::fatalMessageBox("No aircraft", "No aircraft was specified"); + flightgear::fatalMessageBoxWithoutExit("No aircraft", + "No aircraft was specified"); SG_LOG(SG_GENERAL, SG_ALERT, "no aircraft specified"); return false; } @@ -212,10 +213,12 @@ public: try { readProperties(setFile, globals->get_props()); } catch ( const sg_exception &e ) { - SG_LOG(SG_INPUT, SG_ALERT, "Error reading aircraft: " << e.getFormattedMessage()); - flightgear::fatalMessageBox("Error reading aircraft", - "An error occured reading the requested aircraft (" + aircraft + ")", - e.getFormattedMessage()); + SG_LOG(SG_INPUT, SG_ALERT, + "Error reading aircraft: " << e.getFormattedMessage()); + flightgear::fatalMessageBoxWithoutExit( + "Error reading aircraft", + "An error occured reading the requested aircraft (" + aircraft + ")", + e.getFormattedMessage()); return false; } // apply state after the -set.xml, but before any options are are set @@ -224,9 +227,11 @@ public: } else { SG_LOG(SG_GENERAL, SG_ALERT, "aircraft '" << _searchAircraft << "' not found in specified dir:" << aircraftDir); - flightgear::fatalMessageBox("Aircraft not found", - "The requested aircraft '" + aircraft + "' could not be found in the specified location.", - aircraftDir); + flightgear::fatalMessageBoxWithoutExit( + "Aircraft not found", + "The requested aircraft (" + aircraft + ") could not be found " + "in the specified location.", + aircraftDir); return false; } } @@ -245,10 +250,12 @@ public: } if (_foundPath.isNull()) { - SG_LOG(SG_GENERAL, SG_ALERT, "Cannot find specified aircraft: " << aircraft ); - flightgear::fatalMessageBox("Aircraft not found", - "The requested aircraft '" + aircraft + "' could not be found in any of the search paths"); - + SG_LOG(SG_GENERAL, SG_ALERT, + "Cannot find the specified aircraft: '" << aircraft << "'"); + flightgear::fatalMessageBoxWithoutExit( + "Aircraft not found", + "The requested aircraft (" + aircraft + ") could not be found " + "in any of the search paths."); return false; } @@ -262,10 +269,12 @@ public: try { readProperties(_foundPath, globals->get_props()); } catch ( const sg_exception &e ) { - SG_LOG(SG_INPUT, SG_ALERT, "Error reading aircraft: " << e.getFormattedMessage()); - flightgear::fatalMessageBox("Error reading aircraft", - "An error occured reading the requested aircraft (" + aircraft + ")", - e.getFormattedMessage()); + SG_LOG(SG_INPUT, SG_ALERT, + "Error reading aircraft: " << e.getFormattedMessage()); + flightgear::fatalMessageBoxWithoutExit( + "Error reading aircraft", + "An error occured reading the requested aircraft (" + aircraft + ")", + e.getFormattedMessage()); return false; } @@ -351,12 +360,11 @@ static SGPath platformDefaultDataPath() SGPath appDataPath = SGPath::fromEnv("APPDATA"); if (appDataPath.isNull()) { - flightgear::fatalMessageBox( + flightgear::fatalMessageBoxThenExit( "FlightGear", "Unable to get the value of APPDATA.", "FlightGear is unable to retrieve the value of the APPDATA environment " "variable. This is quite unexpected on Windows platforms, and FlightGear " "can't continue its execution without this value, sorry."); - exit(EXIT_FAILURE); } return appDataPath / "flightgear.org"; @@ -384,9 +392,10 @@ bool fgInitHome() } if (!fgHome.exists()) { - flightgear::fatalMessageBox("Problem setting up user data", - "Unable to create the user-data storage folder at: '" - + dataPath.utf8Str() + "'"); + flightgear::fatalMessageBoxWithoutExit( + "Problem setting up user data", + "Unable to create the user-data storage folder at '" + + dataPath.utf8Str() + "'."); return false; } @@ -425,9 +434,9 @@ bool fgInitHome() fgSetBool("/sim/fghome-readonly", true); return true; } - - char buf[16]; - std::string ps = pidPath.local8BitStr(); + + char buf[16]; + std::string ps = pidPath.local8BitStr(); // do open+unlink trick to the file is deleted on exit, even if we // crash or exit(-1) @@ -439,14 +448,17 @@ bool fgInitHome() result = false; } - if (!result) { - flightgear::fatalMessageBox("File permissions problem", - "Can't write to user-data storage folder, check file permissions and FG_HOME.", - "User-data at:" + dataPath.utf8Str()); - } + if (!result) { + flightgear::fatalMessageBoxWithoutExit( + "File permissions problem", + "Can't write to user-data storage folder, check file permissions " + "and FG_HOME.", + "User-data at '" + dataPath.utf8Str() + "'."); + return false; + } #endif fgSetBool("/sim/fghome-readonly", false); - return result; + return result; } // Read in configuration (file and command line) @@ -493,11 +505,12 @@ int fgInitConfig ( int argc, char **argv, bool reinit ) SG_LOG(SG_GENERAL, SG_INFO, "Reading global defaults"); SGPath defaultsXML = globals->get_fg_root() / "defaults.xml"; if (!defaultsXML.exists()) { - flightgear::fatalMessageBox("Missing files", - "Couldn't load an essential simulator data file", - defaultsXML.utf8Str()); + flightgear::fatalMessageBoxThenExit( + "Missing file", + "Couldn't load an essential simulator data file.", + defaultsXML.utf8Str()); } - + fgLoadProps("defaults.xml", globals->get_props()); SG_LOG(SG_GENERAL, SG_INFO, "Finished Reading global defaults"); @@ -582,9 +595,10 @@ int fgInitAircraft(bool reinit) } else { #if 0 // naturally the better option would be to on-demand install it! - flightgear::fatalMessageBox("Aircraft not installed", - "Requested aircraft is not currently installed.", - aircraftId); + flightgear::fatalMessageBoxWithoutExit( + "Aircraft not installed", + "Requested aircraft is not currently installed.", + aircraftId); return flightgear::FG_OPTIONS_ERROR; #endif diff --git a/src/Main/options.cxx b/src/Main/options.cxx index 1a4febe75..85eedcc84 100644 --- a/src/Main/options.cxx +++ b/src/Main/options.cxx @@ -2079,25 +2079,31 @@ void Options::init(int argc, char **argv, const SGPath& appDataPath) // system.fgfsrc is disabled, as we no longer allow anything in fgdata to set // fg-root/fg-home/fg-aircraft and hence control what files Nasal can access - std::string name_for_error = config.utf8Str(); + std::string nameForError = config.utf8Str(); if( ! hostname.empty() ) { config = globals->get_fg_root(); config.append( "system.fgfsrc" ); config.concat( "." ); config.concat( hostname ); if (config.exists()) { - flightgear::fatalMessageBox("Unsupported configuration", - "You have a " + config.utf8Str() + " file, which is no longer processed for security reasons", - "If you created this file intentionally, please move it to " + name_for_error); + flightgear::fatalMessageBoxThenExit( + "Unsupported configuration", + "You have a '" + config.utf8Str() + "' file, which is no longer " + "processed for security reasons.", + "If you created this file intentionally, please move it to '" + + nameForError + "'."); } } config = globals->get_fg_root(); config.append( "system.fgfsrc" ); if (config.exists()) { - flightgear::fatalMessageBox("Unsupported configuration", - "You have a " + config.utf8Str() + " file, which is no longer processed for security reasons", - "If you created this file intentionally, please move it to " + name_for_error); + flightgear::fatalMessageBoxThenExit( + "Unsupported configuration", + "You have a '" + config.utf8Str() + "' file, which is no longer " + "processed for security reasons.", + "If you created this file intentionally, please move it to '" + + nameForError + "'."); } } @@ -2818,26 +2824,24 @@ void Options::setupRoot(int argc, char **argv) } #else SG_UNUSED(usingDefaultRoot); - + // validate it if (base_version.empty()) { - flightgear::fatalMessageBox("Base package not found", - "Required data files not found, check your installation.", - "Looking for base-package files at: '" + root.str() + "'"); - - exit(-1); + flightgear::fatalMessageBoxThenExit( + "Base package not found", + "Required data files not found, please check your installation.", + "Looking for base-package files at: '" + root.str() + "'"); } // only compare major and minor version, not the patch level. const int versionComp = simgear::strutils::compare_versions(FLIGHTGEAR_VERSION, base_version, 2); if (versionComp != 0) { - flightgear::fatalMessageBox("Base package version mismatch", - "Version check failed: please check your installation.", - "Found data files for version '" + base_version + - "' at '" + globals->get_fg_root().str() + "', version '" - + std::string(FLIGHTGEAR_VERSION) + "' is required."); - - exit(-1); + flightgear::fatalMessageBoxThenExit( + "Base package version mismatch", + "Version check failed, please check your installation.", + "Found data files for version '" + base_version + "' at '" + + globals->get_fg_root().str() + "', version '" + + std::string(FLIGHTGEAR_VERSION) + "' is required."); } #endif } diff --git a/src/Main/util.cxx b/src/Main/util.cxx index fb0a1668e..96ab82bd6 100644 --- a/src/Main/util.cxx +++ b/src/Main/util.cxx @@ -85,10 +85,9 @@ void fgInitAllowedPaths() // Forbid using this version of fgValidatePath() with older // (not normalizing non-existent files) versions of realpath(), // as that would be a security hole - flightgear::fatalMessageBox("Nasal initialization error", - "Version mismatch - please update simgear", - ""); - exit(-1); + flightgear::fatalMessageBoxThenExit( + "Nasal initialization error", + "Version mismatch - please update simgear"); } read_allowed_paths.clear(); write_allowed_paths.clear(); @@ -102,10 +101,10 @@ void fgInitAllowedPaths() // if we get the initialization order wrong, better to have an // obvious error than a can-read-everything security hole... if (it->isNull()) { - flightgear::fatalMessageBox("Nasal initialization error", - "Empty string in FG_ROOT, FG_HOME, FG_AIRCRAFT, FG_SCENERY or --allow-nasal-read", - "or fgInitAllowedPaths() called too early"); - exit(-1); + flightgear::fatalMessageBoxThenExit( + "Nasal initialization error", + "Empty string in FG_ROOT, FG_HOME, FG_AIRCRAFT, FG_SCENERY or " + "--allow-nasal-read, or fgInitAllowedPaths() called too early"); } read_allowed_paths.push_back(it->realpath().utf8Str() + "/*"); read_allowed_paths.push_back(it->realpath().utf8Str()); @@ -131,10 +130,12 @@ void fgInitAllowedPaths() !fgValidatePath(homePath + "\\..\\no.log",false).isNull() || fgValidatePath(homePath + "/aircraft-data/yes..xml",true).isNull() || fgValidatePath(homePath + "/.\\yes.bmp",false).isNull()) { - flightgear::fatalMessageBox("Nasal initialization error", - "The FG_HOME directory must not be inside any of the FG_ROOT, FG_AIRCRAFT, FG_SCENERY or --allow-nasal-read directories", - "(check that you have not accidentally included an extra :, as an empty part means the current directory)"); - exit(-1); + flightgear::fatalMessageBoxThenExit( + "Nasal initialization error", + "The FG_HOME directory must not be inside any of the FG_ROOT, " + "FG_AIRCRAFT, FG_SCENERY or --allow-nasal-read directories", + "(check that you have not accidentally included an extra ':', " + "as an empty part means the current directory)"); } } diff --git a/src/Navaids/NavDataCache.cxx b/src/Navaids/NavDataCache.cxx index fb3c63b0a..fd7acd54d 100644 --- a/src/Navaids/NavDataCache.cxx +++ b/src/Navaids/NavDataCache.cxx @@ -1229,9 +1229,10 @@ NavDataCache::NavDataCache() if (t == (MAX_TRIES - 1)) { // final attempt still failed, we are busted - flightgear::fatalMessageBox("Unable to open navigation cache", - std::string("The navigation data cache could not be opened:") + e.getMessage(), - e.getOrigin()); + flightgear::fatalMessageBoxThenExit( + "Unable to open navigation cache", + std::string("The navigation data cache could not be opened:") + + e.getMessage(), e.getOrigin()); } d.reset(); @@ -1241,9 +1242,10 @@ NavDataCache::NavDataCache() bool ok = homePath.remove(); if (!ok) { SG_LOG(SG_NAVCACHE, SG_ALERT, "NavCache: failed to remove previous cache file"); - flightgear::fatalMessageBox("Unable to re-create navigation cache", - "Attempting to remove the old cache failed.", - "Location: " + homePath.utf8Str()); + flightgear::fatalMessageBoxThenExit( + "Unable to re-create navigation cache", + "Attempting to remove the old cache failed.", + "Location: " + homePath.utf8Str()); } } } diff --git a/tests/testStubs.cxx b/tests/testStubs.cxx index 84b9522d3..8411a136a 100644 --- a/tests/testStubs.cxx +++ b/tests/testStubs.cxx @@ -119,6 +119,14 @@ namespace flightgear { return MSG_BOX_OK; } + + [[noreturn]] void fatalMessageBoxThenExit(const std::string& caption, + const std::string& msg, + const std::string& moreText, + int exitStatus) + { + exit(exitStatus); + } } #ifdef __APPLE__