From bd0de1386e1c7f9b875d52290de0d561c8d56bc9 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 2 Nov 2016 21:59:09 +0300 Subject: [PATCH 1/2] Fix exit codes: - `--help`, `--version` etc should exit with `0` i.e. no error ("not enough args" case should still trigger an error) - error reading config file should exit with `1` Slightly refactor AppInitRPC/AppInitRawTx to return standard exit codes (EXIT_FAILURE/EXIT_SUCCESS) or CONTINUE_EXECUTION (-1) --- src/bitcoin-cli.cpp | 28 +++++++++++++++++++--------- src/bitcoin-tx.cpp | 20 +++++++++++++++----- src/bitcoind.cpp | 2 +- src/qt/bitcoin.cpp | 4 ++-- 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 2d66448d8..380b8dadd 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -28,6 +28,7 @@ using namespace std; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; +static const int CONTINUE_EXECUTION=-1; std::string HelpMessageCli() { @@ -67,7 +68,11 @@ public: }; -static bool AppInitRPC(int argc, char* argv[]) +// +// This function returns either one of EXIT_ codes when it's expected to stop the process or +// CONTINUE_EXECUTION when it's expected to continue further. +// +static int AppInitRPC(int argc, char* argv[]) { // // Parameters @@ -85,31 +90,35 @@ static bool AppInitRPC(int argc, char* argv[]) } fprintf(stdout, "%s", strUsage.c_str()); - return false; + if (argc < 2) { + fprintf(stderr, "Error: too few parameters\n"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } if (!boost::filesystem::is_directory(GetDataDir(false))) { fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", mapArgs["-datadir"].c_str()); - return false; + return EXIT_FAILURE; } try { ReadConfigFile(GetArg("-conf", BITCOIN_CONF_FILENAME), mapArgs, mapMultiArgs); } catch (const std::exception& e) { fprintf(stderr,"Error reading configuration file: %s\n", e.what()); - return false; + return EXIT_FAILURE; } // Check for -testnet or -regtest parameter (BaseParams() calls are only valid after this clause) try { SelectBaseParams(ChainNameFromCommandLine()); } catch (const std::exception& e) { fprintf(stderr, "Error: %s\n", e.what()); - return false; + return EXIT_FAILURE; } if (GetBoolArg("-rpcssl", false)) { fprintf(stderr, "Error: SSL mode for RPC (-rpcssl) is no longer supported.\n"); - return false; + return EXIT_FAILURE; } - return true; + return CONTINUE_EXECUTION; } @@ -354,8 +363,9 @@ int main(int argc, char* argv[]) } try { - if(!AppInitRPC(argc, argv)) - return EXIT_FAILURE; + int ret = AppInitRPC(argc, argv); + if (ret != CONTINUE_EXECUTION) + return ret; } catch (const std::exception& e) { PrintExceptionContinue(&e, "AppInitRPC()"); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index e09afd632..ecc19c44a 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -30,8 +30,13 @@ using namespace std; static bool fCreateBlank; static map registers; +static const int CONTINUE_EXECUTION=-1; -static bool AppInitRawTx(int argc, char* argv[]) +// +// This function returns either one of EXIT_ codes when it's expected to stop the process or +// CONTINUE_EXECUTION when it's expected to continue further. +// +static int AppInitRawTx(int argc, char* argv[]) { // // Parameters @@ -89,9 +94,13 @@ static bool AppInitRawTx(int argc, char* argv[]) strUsage += HelpMessageOpt("set=NAME:JSON-STRING", _("Set register NAME to given JSON-STRING")); fprintf(stdout, "%s", strUsage.c_str()); - return false; + if (argc < 2) { + fprintf(stderr, "Error: too few parameters\n"); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } - return true; + return CONTINUE_EXECUTION; } static void RegisterSetJson(const string& key, const string& rawJson) @@ -678,8 +687,9 @@ int main(int argc, char* argv[]) SetupEnvironment(); try { - if(!AppInitRawTx(argc, argv)) - return EXIT_FAILURE; + int ret = AppInitRawTx(argc, argv); + if (ret != CONTINUE_EXECUTION) + return ret; } catch (const std::exception& e) { PrintExceptionContinue(&e, "AppInitRawTx()"); diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 351463c25..a0d039359 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -92,7 +92,7 @@ bool AppInit(int argc, char* argv[]) } fprintf(stdout, "%s", strUsage.c_str()); - return false; + return true; } try diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 9986af495..fb66ff007 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -573,7 +573,7 @@ int main(int argc, char *argv[]) { HelpMessageDialog help(NULL, mapArgs.count("-version")); help.showOrPrint(); - return 1; + return 0; } /// 5. Now that settings and translations are available, ask user for data directory @@ -594,7 +594,7 @@ int main(int argc, char *argv[]) } catch (const std::exception& e) { QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what())); - return false; + return 1; } /// 7. Determine network (and switch to network specific options) From 4441018d0860fce64ee74fa78da79bbb21114ca9 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 7 Nov 2016 21:31:38 +0300 Subject: [PATCH 2/2] Every main()/exit() should return/use one of EXIT_ codes instead of magic numbers --- src/bitcoind.cpp | 4 ++-- src/qt/bitcoin.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index a0d039359..3352a76de 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -126,7 +126,7 @@ bool AppInit(int argc, char* argv[]) if (fCommandLine) { fprintf(stderr, "Error: There is no RPC client functionality in bitcoind anymore. Use the bitcoin-cli utility instead.\n"); - exit(1); + exit(EXIT_FAILURE); } if (GetBoolArg("-daemon", false)) { @@ -177,5 +177,5 @@ int main(int argc, char* argv[]) // Connect bitcoind signal handlers noui_connect(); - return (AppInit(argc, argv) ? 0 : 1); + return (AppInit(argc, argv) ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index fb66ff007..c828234f4 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -496,7 +496,7 @@ void BitcoinApplication::shutdownResult(int retval) void BitcoinApplication::handleRunawayException(const QString &message) { QMessageBox::critical(0, "Runaway exception", BitcoinGUI::tr("A fatal error occurred. Bitcoin can no longer continue safely and will quit.") + QString("\n\n") + message); - ::exit(1); + ::exit(EXIT_FAILURE); } WId BitcoinApplication::getMainWinId() const @@ -573,13 +573,13 @@ int main(int argc, char *argv[]) { HelpMessageDialog help(NULL, mapArgs.count("-version")); help.showOrPrint(); - return 0; + return EXIT_SUCCESS; } /// 5. Now that settings and translations are available, ask user for data directory // User language is set up: pick a data directory if (!Intro::pickDataDirectory()) - return 0; + return EXIT_SUCCESS; /// 6. Determine availability of data directory and parse bitcoin.conf /// - Do not call GetDataDir(true) before this step finishes @@ -587,14 +587,14 @@ int main(int argc, char *argv[]) { QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(mapArgs["-datadir"]))); - return 1; + return EXIT_FAILURE; } try { ReadConfigFile(GetArg("-conf", BITCOIN_CONF_FILENAME), mapArgs, mapMultiArgs); } catch (const std::exception& e) { QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what())); - return 1; + return EXIT_FAILURE; } /// 7. Determine network (and switch to network specific options) @@ -608,7 +608,7 @@ int main(int argc, char *argv[]) SelectParams(ChainNameFromCommandLine()); } catch(std::exception &e) { QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: %1").arg(e.what())); - return 1; + return EXIT_FAILURE; } #ifdef ENABLE_WALLET // Parse URIs on command line -- this can affect Params() @@ -630,7 +630,7 @@ int main(int argc, char *argv[]) // - Do this after creating app and setting up translations, so errors are // translated properly. if (PaymentServer::ipcSendCommandLine()) - exit(0); + exit(EXIT_SUCCESS); // Start up the payment server early, too, so impatient users that click on // bitcoin: links repeatedly have their payment requests routed to this process: