From 192325a77d593e404e74ef5e204aed8801b4e66f Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 28 Sep 2022 18:02:23 +0000 Subject: [PATCH 1/2] kernel: move RunCommandParseJSON to its own file Because libbitcoinkernel does not include this new object, this has the side-effect of eliminating the unnecessary boost::process dependency. --- src/Makefile.am | 2 ++ src/external_signer.cpp | 2 +- src/test/system_tests.cpp | 2 +- src/util/run_command.cpp | 64 +++++++++++++++++++++++++++++++++++++++ src/util/run_command.h | 21 +++++++++++++ src/util/system.cpp | 51 ------------------------------- src/util/system.h | 8 ----- 7 files changed, 89 insertions(+), 61 deletions(-) create mode 100644 src/util/run_command.cpp create mode 100644 src/util/run_command.h diff --git a/src/Makefile.am b/src/Makefile.am index 6364e00d1e..f5cf8cbca5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -284,6 +284,7 @@ BITCOIN_CORE_H = \ util/rbf.h \ util/readwritefile.h \ util/result.h \ + util/run_command.h \ util/serfloat.h \ util/settings.h \ util/sock.h \ @@ -680,6 +681,7 @@ libbitcoin_util_a_SOURCES = \ util/fees.cpp \ util/getuniquepath.cpp \ util/hasher.cpp \ + util/run_command.cpp \ util/sock.cpp \ util/syserror.cpp \ util/system.cpp \ diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 094314e878..d1eec2fd61 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -5,8 +5,8 @@ #include #include #include +#include #include -#include #include #include diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index f160bb08a5..ae6800c67c 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include -#include +#include #include #ifdef ENABLE_EXTERNAL_SIGNER diff --git a/src/util/run_command.cpp b/src/util/run_command.cpp new file mode 100644 index 0000000000..90d677f512 --- /dev/null +++ b/src/util/run_command.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#if defined(HAVE_CONFIG_H) +#include +#endif + +#include + +#include +#include + +#ifdef ENABLE_EXTERNAL_SIGNER +#if defined(__GNUC__) +// Boost 1.78 requires the following workaround. +// See: https://github.com/boostorg/process/issues/235 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wnarrowing" +#endif +#include +#if defined(__GNUC__) +#pragma GCC diagnostic pop +#endif +#endif // ENABLE_EXTERNAL_SIGNER + +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) +{ +#ifdef ENABLE_EXTERNAL_SIGNER + namespace bp = boost::process; + + UniValue result_json; + bp::opstream stdin_stream; + bp::ipstream stdout_stream; + bp::ipstream stderr_stream; + + if (str_command.empty()) return UniValue::VNULL; + + bp::child c( + str_command, + bp::std_out > stdout_stream, + bp::std_err > stderr_stream, + bp::std_in < stdin_stream + ); + if (!str_std_in.empty()) { + stdin_stream << str_std_in << std::endl; + } + stdin_stream.pipe().close(); + + std::string result; + std::string error; + std::getline(stdout_stream, result); + std::getline(stderr_stream, error); + + c.wait(); + const int n_error = c.exit_code(); + if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); + if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); + + return result_json; +#else + throw std::runtime_error("Compiled without external signing support (required for external signing)."); +#endif // ENABLE_EXTERNAL_SIGNER +} diff --git a/src/util/run_command.h b/src/util/run_command.h new file mode 100644 index 0000000000..afe5d831c6 --- /dev/null +++ b/src/util/run_command.h @@ -0,0 +1,21 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_RUN_COMMAND_H +#define BITCOIN_UTIL_RUN_COMMAND_H + +#include + +class UniValue; + +/** + * Execute a command which returns JSON, and parse the result. + * + * @param str_command The command to execute, including any arguments + * @param str_std_in string to pass to stdin + * @return parsed JSON + */ +UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); + +#endif // BITCOIN_UTIL_RUN_COMMAND_H diff --git a/src/util/system.cpp b/src/util/system.cpp index c3c6cbfef6..ce5d846eb9 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -5,19 +5,6 @@ #include -#ifdef ENABLE_EXTERNAL_SIGNER -#if defined(__GNUC__) -// Boost 1.78 requires the following workaround. -// See: https://github.com/boostorg/process/issues/235 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wnarrowing" -#endif -#include -#if defined(__GNUC__) -#pragma GCC diagnostic pop -#endif -#endif // ENABLE_EXTERNAL_SIGNER - #include #include #include @@ -1332,44 +1319,6 @@ void runCommand(const std::string& strCommand) } #endif -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) -{ -#ifdef ENABLE_EXTERNAL_SIGNER - namespace bp = boost::process; - - UniValue result_json; - bp::opstream stdin_stream; - bp::ipstream stdout_stream; - bp::ipstream stderr_stream; - - if (str_command.empty()) return UniValue::VNULL; - - bp::child c( - str_command, - bp::std_out > stdout_stream, - bp::std_err > stderr_stream, - bp::std_in < stdin_stream - ); - if (!str_std_in.empty()) { - stdin_stream << str_std_in << std::endl; - } - stdin_stream.pipe().close(); - - std::string result; - std::string error; - std::getline(stdout_stream, result); - std::getline(stderr_stream, error); - - c.wait(); - const int n_error = c.exit_code(); - if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); - if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); - - return result_json; -#else - throw std::runtime_error("Compiled without external signing support (required for external signing)."); -#endif // ENABLE_EXTERNAL_SIGNER -} void SetupEnvironment() { diff --git a/src/util/system.h b/src/util/system.h index c8e1de6700..29629e547e 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -107,14 +107,6 @@ std::string ShellEscape(const std::string& arg); #if HAVE_SYSTEM void runCommand(const std::string& strCommand); #endif -/** - * Execute a command which returns JSON, and parse the result. - * - * @param str_command The command to execute, including any arguments - * @param str_std_in string to pass to stdin - * @return parsed JSON - */ -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); /** * Most paths passed as configuration arguments are treated as relative to From 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 4 Oct 2022 18:15:53 +0000 Subject: [PATCH 2/2] refactor: move run_command from util to common Quoting ryanofsky: "util can be the library for things included in the kernel which the kernel can depend on, and common can be the library for other code that needs to be shared internally, but should not be part of the kernel or shared externally." --- src/Makefile.am | 6 +++--- src/{util => common}/run_command.cpp | 2 +- src/{util => common}/run_command.h | 6 +++--- src/external_signer.cpp | 2 +- src/test/system_tests.cpp | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) rename src/{util => common}/run_command.cpp (98%) rename src/{util => common}/run_command.h (82%) diff --git a/src/Makefile.am b/src/Makefile.am index f5cf8cbca5..592f06aec6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -133,6 +133,7 @@ BITCOIN_CORE_H = \ clientversion.h \ coins.h \ common/bloom.h \ + common/run_command.h \ compat/assumptions.h \ compat/byteswap.h \ compat/compat.h \ @@ -284,7 +285,6 @@ BITCOIN_CORE_H = \ util/rbf.h \ util/readwritefile.h \ util/result.h \ - util/run_command.h \ util/serfloat.h \ util/settings.h \ util/sock.h \ @@ -617,7 +617,7 @@ libbitcoin_consensus_a_SOURCES = \ version.h # common: shared between bitcoind, and bitcoin-qt and non-server tools -libbitcoin_common_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_common_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libbitcoin_common_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_common_a_SOURCES = \ base58.cpp \ @@ -625,6 +625,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ common/bloom.cpp \ + common/run_command.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -681,7 +682,6 @@ libbitcoin_util_a_SOURCES = \ util/fees.cpp \ util/getuniquepath.cpp \ util/hasher.cpp \ - util/run_command.cpp \ util/sock.cpp \ util/syserror.cpp \ util/system.cpp \ diff --git a/src/util/run_command.cpp b/src/common/run_command.cpp similarity index 98% rename from src/util/run_command.cpp rename to src/common/run_command.cpp index 90d677f512..6ad9f75b5d 100644 --- a/src/util/run_command.cpp +++ b/src/common/run_command.cpp @@ -6,7 +6,7 @@ #include #endif -#include +#include #include #include diff --git a/src/util/run_command.h b/src/common/run_command.h similarity index 82% rename from src/util/run_command.h rename to src/common/run_command.h index afe5d831c6..2fbdc071ee 100644 --- a/src/util/run_command.h +++ b/src/common/run_command.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_RUN_COMMAND_H -#define BITCOIN_UTIL_RUN_COMMAND_H +#ifndef BITCOIN_COMMON_RUN_COMMAND_H +#define BITCOIN_COMMON_RUN_COMMAND_H #include @@ -18,4 +18,4 @@ class UniValue; */ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); -#endif // BITCOIN_UTIL_RUN_COMMAND_H +#endif // BITCOIN_COMMON_RUN_COMMAND_H diff --git a/src/external_signer.cpp b/src/external_signer.cpp index d1eec2fd61..0e582629f7 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -3,9 +3,9 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include -#include #include #include diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index ae6800c67c..11f4be7fef 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include -#include +#include #include #ifdef ENABLE_EXTERNAL_SIGNER