From b6b2ce7adae12b87ef99e9e956a02aa89c80f806 Mon Sep 17 00:00:00 2001 From: Hayodea Hakol Date: Wed, 13 Aug 2025 09:43:34 -0400 Subject: [PATCH] Cmdline: use exceptions for control flow This is generally frowned upon but it makes this code 10x cleaner. We handle commandLine usage msg printing by using exceptions for control flow. This allows us to centralize the logic for killing the Mind threads in one place. At least with respect to printing the usage msg. --- smocore/include/opts.h | 31 ++++++++++++++++++ smocore/marionette/marionette.cpp | 53 ++++++++++++++++--------------- smocore/opts.cpp | 34 ++++++++++++++++---- 3 files changed, 86 insertions(+), 32 deletions(-) diff --git a/smocore/include/opts.h b/smocore/include/opts.h index 9bec880..b1988a8 100644 --- a/smocore/include/opts.h +++ b/smocore/include/opts.h @@ -4,6 +4,8 @@ #include #include #include +#include +#include // Define a class to hold the options and parse arguments class OptionParser @@ -23,6 +25,14 @@ public: } public: + class Exception : public std::exception + { + public: + Exception() = default; + ~Exception() override = default; + const char* what() const noexcept override = 0; + }; + std::string argv0; std::vector senseApiLibPath; std::vector senseApiLibs; @@ -33,4 +43,25 @@ public: static struct option longOptions[]; }; +class OptionsParserError +: public std::invalid_argument, public OptionParser::Exception +{ +public: + OptionsParserError( + const std::string& errorMessage, const OptionParser& parser); + + const char* what() const noexcept override; +}; + +class JustPrintUsageNoError +: public OptionParser::Exception +{ +public: + explicit JustPrintUsageNoError(const OptionParser& parser); + const char* what() const noexcept override; + +private: + std::string message; +}; + #endif // OPTS_H diff --git a/smocore/marionette/marionette.cpp b/smocore/marionette/marionette.cpp index c924f62..277e6ca 100644 --- a/smocore/marionette/marionette.cpp +++ b/smocore/marionette/marionette.cpp @@ -8,6 +8,7 @@ #include #include #include +#include namespace smo { @@ -55,34 +56,15 @@ void ComponentThread::marionetteMain(ComponentThread& self) std::cout << __func__ << ": " << PACKAGE_NAME << " " << PACKAGE_VERSION << std::endl; - try { - options.parseArguments( - crtCommandLineArgs.argc, crtCommandLineArgs.argv, - crtCommandLineArgs.envp); + options.parseArguments( + crtCommandLineArgs.argc, crtCommandLineArgs.argv, + crtCommandLineArgs.envp); - std::cout << __func__ << ": " << options.stringifyOptions() - << std::endl; - } - catch (const std::invalid_argument& e) - { - std::cerr << __func__ << ": Exception occurred: " << e.what() - << '\n'; + std::cout << __func__ << ": " << options.stringifyOptions() + << std::endl; - options.printUsage = true; - mrntt::exitCode = EXIT_FAILURE; - } - - if (options.printUsage) - { - /* We could make RAII guard classes to always shutdown the mind - * threads properly in these pre-event loop situations. - */ - mind.finalizeReq([]{ - mrntt::mrntt->getIoService().stop(); - }); - self.getIoService().reset(); - self.getIoService().run(); - return; + if (options.printUsage) { + throw JustPrintUsageNoError(options); } self.getIoService().post([]() @@ -130,6 +112,25 @@ void ComponentThread::marionetteMain(ComponentThread& self) std::cout << __func__ << ": Exited event loop" << "\n"; shutdownSalmanoff(); } + catch (const OptionParser::Exception& e) + { + std::ostream &out = std::cout; + std::string outUsageMsg; + + if (typeid(e) == typeid(OptionsParserError)) + { + mrntt::exitCode = EXIT_FAILURE; + outUsageMsg = std::string(__func__) + ": "; + } + + out << outUsageMsg << e.what() << std::endl; + mind.finalizeReq([]{ + mrntt::mrntt->getIoService().stop(); + }); + self.getIoService().reset(); + self.getIoService().run(); + return; + } catch (const std::exception& e) { std::cerr << __func__ << ": Exception occurred: " << e.what() diff --git a/smocore/opts.cpp b/smocore/opts.cpp index d849804..eefd500 100644 --- a/smocore/opts.cpp +++ b/smocore/opts.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -8,6 +9,28 @@ #include +OptionsParserError::OptionsParserError( + const std::string& errorMessage, const OptionParser& parser +) +: std::invalid_argument(errorMessage + "\n" + parser.getUsage()) +{ +} + +const char* OptionsParserError::what() const noexcept +{ + return std::invalid_argument::what(); +} + +JustPrintUsageNoError::JustPrintUsageNoError(const OptionParser& parser) +: message(parser.getUsage()) +{ +} + +const char* JustPrintUsageNoError::what() const noexcept +{ + return message.c_str(); +} + struct option OptionParser::longOptions[] = { {"devicespec", required_argument, 0, 's'}, {"spec", required_argument, 0, 's'}, @@ -59,9 +82,9 @@ void OptionParser::parseArguments(int argc, char *argv[], char **envp) if (stat(optarg, &info) != 0 || !(info.st_mode & S_IFDIR)) { - throw std::invalid_argument( + throw OptionsParserError( std::string(__func__) + " - The specified path is not a " - "directory: " + optarg); + "directory: " + optarg, *this); } senseApiLibPath.push_back(optarg); @@ -71,12 +94,11 @@ void OptionParser::parseArguments(int argc, char *argv[], char **envp) verbose = true; break; case 'h': - printUsage = true; - return; + throw JustPrintUsageNoError(*this); case '?': - throw std::invalid_argument( + throw OptionsParserError( std::string(__func__) + " - Invalid argument encountered: " - + std::string(argv[optind - 1])); + + std::string(argv[optind - 1]), *this); } } }