foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-04 13:55:23 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#27632: Raise on invalid -debug and -loglevel config options

Browse source 
daa5a658c0 refactor: rename BCLog::BLOCKSTORE to BLOCKSTORAGE (Jon Atack)
cf622b214b doc: release note re raising on invalid -debug/debugexclude/loglevel (Jon Atack)
6cb1c66041 init: remove config option names from translated -loglevel strings (Jon Atack)
2547829272 test: -loglevel raises on invalid values (Jon Atack)
a9c295888b init: raise on invalid loglevel config option (Jon Atack)
b0c3995393 test: -debug and -debugexclude raise on invalid values (Jon Atack)
4c3c19d943 init: raise on invalid debug/debugexclude config options (Jon Atack)

Pull request description:

  and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums.

  Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458.

ACKs for top commit:
  achow101:
    ACK daa5a658c0
  ryanofsky:
    Code review ACK daa5a658c0. Just translated string template cleanup since last review
  pinheadmz:
    re-ACK daa5a658c0

Tree-SHA512: 4c107a93d8e8ce4e2ee81d44aec672526ca354ec390b241221067f68204beac8b4ba7a65748bcfa124ff2245c4307fa9243ec4fe0b464d0fa69c787fb322c3cc
This commit is contained in:
Andrew Chow 2023-06-20 13:41:26 -04:00
commit e4bbfb2d49
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
9 changed files with 67 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
doc/release-notes-27632.md Normal file
View file

@ -0,0 +1,5 @@
Updated settings
----------------
- Passing an invalid `-debug`, `-debugexclude`, or `-loglevel` logging configuration
option now raises an error, rather than logging an easily missed warning. (#27632)

7
src/init.cpp
View file

@ -77,6 +77,7 @@
#include <util/fs.h>
#include <util/fs_helpers.h>
#include <util/moneystr.h>
#include <util/result.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/syscall_sandbox.h>
@ -951,8 +952,10 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections));
// ********************************************************* Step 3: parameter-to-internal-flags
init::SetLoggingCategories(args);
init::SetLoggingLevel(args);
auto result = init::SetLoggingCategories(args);
if (!result) return InitError(util::ErrorString(result));
result = init::SetLoggingLevel(args);
if (!result) return InitError(util::ErrorString(result));
nConnectTimeout = args.GetIntArg("-timeout", DEFAULT_CONNECT_TIMEOUT);
if (nConnectTimeout <= 0) {

15
src/init/common.cpp
View file

@ -13,6 +13,7 @@
#include <tinyformat.h>
#include <util/fs.h>
#include <util/fs_helpers.h>
#include <util/result.h>
#include <util/string.h>
#include <util/time.h>
#include <util/translation.h>
@ -58,27 +59,28 @@ void SetLoggingOptions(const ArgsManager& args)
fLogIPs = args.GetBoolArg("-logips", DEFAULT_LOGIPS);
}
void SetLoggingLevel(const ArgsManager& args)
util::Result<void> SetLoggingLevel(const ArgsManager& args)
{
if (args.IsArgSet("-loglevel")) {
for (const std::string& level_str : args.GetArgs("-loglevel")) {
if (level_str.find_first_of(':', 3) == std::string::npos) {
// user passed a global log level, i.e. -loglevel=<level>
if (!LogInstance().SetLogLevel(level_str)) {
InitWarning(strprintf(_("Unsupported global logging level -loglevel=%s. Valid values: %s."), level_str, LogInstance().LogLevelsString()));
return util::Error{strprintf(_("Unsupported global logging level %s=%s. Valid values: %s."), "-loglevel", level_str, LogInstance().LogLevelsString())};
}
} else {
// user passed a category-specific log level, i.e. -loglevel=<category>:<level>
const auto& toks = SplitString(level_str, ':');
if (!(toks.size() == 2 && LogInstance().SetCategoryLogLevel(toks[0], toks[1]))) {
InitWarning(strprintf(_("Unsupported category-specific logging level -loglevel=%s. Expected -loglevel=<category>:<loglevel>. Valid categories: %s. Valid loglevels: %s."), level_str, LogInstance().LogCategoriesString(), LogInstance().LogLevelsString()));
return util::Error{strprintf(_("Unsupported category-specific logging level %1$s=%2$s. Expected %1$s=<category>:<loglevel>. Valid categories: %3$s. Valid loglevels: %4$s."), "-loglevel", level_str, LogInstance().LogCategoriesString(), LogInstance().LogLevelsString())};
}
}
}
}
return {};
}
void SetLoggingCategories(const ArgsManager& args)
util::Result<void> SetLoggingCategories(const ArgsManager& args)
{
if (args.IsArgSet("-debug")) {
// Special-case: if -debug=0/-nodebug is set, turn off debugging messages
@ -88,7 +90,7 @@ void SetLoggingCategories(const ArgsManager& args)
[](std::string cat){return cat == "0" || cat == "none";})) {
for (const auto& cat : categories) {
if (!LogInstance().EnableCategory(cat)) {
InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debug", cat));
return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debug", cat)};
}
}
}
@ -97,9 +99,10 @@ void SetLoggingCategories(const ArgsManager& args)
// Now remove the logging categories which were explicitly excluded
for (const std::string& cat : args.GetArgs("-debugexclude")) {
if (!LogInstance().DisableCategory(cat)) {
InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat));
return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat)};
}
}
return {};
}
bool StartLogging(const ArgsManager& args)

6
src/init/common.h
View file

@ -8,13 +8,15 @@
#ifndef BITCOIN_INIT_COMMON_H
#define BITCOIN_INIT_COMMON_H
#include <util/result.h>
class ArgsManager;
namespace init {
void AddLoggingArgs(ArgsManager& args);
void SetLoggingOptions(const ArgsManager& args);
void SetLoggingCategories(const ArgsManager& args);
void SetLoggingLevel(const ArgsManager& args);
[[nodiscard]] util::Result<void> SetLoggingCategories(const ArgsManager& args);
[[nodiscard]] util::Result<void> SetLoggingLevel(const ArgsManager& args);
bool StartLogging(const ArgsManager& args);
void LogPackageVersion();
} // namespace init

4
src/logging.cpp
View file

@ -179,7 +179,7 @@ const CLogCategoryDesc LogCategories[] =
{BCLog::LOCK, "lock"},
#endif
{BCLog::UTIL, "util"},
{BCLog::BLOCKSTORE, "blockstorage"},
{BCLog::BLOCKSTORAGE, "blockstorage"},
{BCLog::TXRECONCILIATION, "txreconciliation"},
{BCLog::SCAN, "scan"},
{BCLog::ALL, "1"},
@ -280,7 +280,7 @@ std::string LogCategoryToStr(BCLog::LogFlags category)
#endif
case BCLog::LogFlags::UTIL:
return "util";
case BCLog::LogFlags::BLOCKSTORE:
case BCLog::LogFlags::BLOCKSTORAGE:
return "blockstorage";
case BCLog::LogFlags::TXRECONCILIATION:
return "txreconciliation";

2
src/logging.h
View file

@ -65,7 +65,7 @@ namespace BCLog {
LOCK = (1 << 24),
#endif
UTIL = (1 << 25),
BLOCKSTORE = (1 << 26),
BLOCKSTORAGE = (1 << 26),
TXRECONCILIATION = (1 << 27),
SCAN = (1 << 28),
ALL = ~(uint32_t)0,

4
src/node/blockstorage.cpp
View file

@ -573,7 +573,7 @@ void BlockManager::UnlinkPrunedFiles(const std::set<int>& setFilesToPrune) const
const bool removed_blockfile{fs::remove(BlockFileSeq().FileName(pos), ec)};
const bool removed_undofile{fs::remove(UndoFileSeq().FileName(pos), ec)};
if (removed_blockfile || removed_undofile) {
LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it);
LogPrint(BCLog::BLOCKSTORAGE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it);
}
}
}
@ -642,7 +642,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne
if ((int)nFile != m_last_blockfile) {
if (!fKnown) {
LogPrint(BCLog::BLOCKSTORE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString());
LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString());
}
FlushBlockFile(!fKnown, finalize_undo);
m_last_blockfile = nFile;

12
src/test/logging_tests.cpp
View file

@ -200,7 +200,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
const char* argv_test[] = {"bitcoind", "-loglevel=debug"};
std::string err;
BOOST_REQUIRE(args.ParseParameters(2, argv_test, err));
init::SetLoggingLevel(args);
auto result = init::SetLoggingLevel(args);
BOOST_REQUIRE(result);
BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug);
}
@ -212,7 +214,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
const char* argv_test[] = {"bitcoind", "-loglevel=net:trace"};
std::string err;
BOOST_REQUIRE(args.ParseParameters(2, argv_test, err));
init::SetLoggingLevel(args);
auto result = init::SetLoggingLevel(args);
BOOST_REQUIRE(result);
BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::DEFAULT_LOG_LEVEL);
const auto& category_levels{LogInstance().CategoryLevels()};
@ -229,7 +233,9 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
const char* argv_test[] = {"bitcoind", "-loglevel=debug", "-loglevel=net:trace", "-loglevel=http:info"};
std::string err;
BOOST_REQUIRE(args.ParseParameters(4, argv_test, err));
init::SetLoggingLevel(args);
auto result = init::SetLoggingLevel(args);
BOOST_REQUIRE(result);
BOOST_CHECK_EQUAL(LogInstance().LogLevel(), BCLog::Level::Debug);
const auto& category_levels{LogInstance().CategoryLevels()};

30
test/functional/feature_logging.py
View file

@ -69,6 +69,36 @@ class LoggingTest(BitcoinTestFramework):
# just sanity check no crash here
self.restart_node(0, [f"-debuglogfile={os.devnull}"])
self.log.info("Test -debug and -debugexclude raise when invalid values are passed")
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(
extra_args=["-debug=abc"],
expected_msg="Error: Unsupported logging category -debug=abc.",
match=ErrorMatch.FULL_REGEX,
)
self.nodes[0].assert_start_raises_init_error(
extra_args=["-debugexclude=abc"],
expected_msg="Error: Unsupported logging category -debugexclude=abc.",
match=ErrorMatch.FULL_REGEX,
)
self.log.info("Test -loglevel raises when invalid values are passed")
self.nodes[0].assert_start_raises_init_error(
extra_args=["-loglevel=abc"],
expected_msg="Error: Unsupported global logging level -loglevel=abc. Valid values: info, debug, trace.",
match=ErrorMatch.FULL_REGEX,
)
self.nodes[0].assert_start_raises_init_error(
extra_args=["-loglevel=net:abc"],
expected_msg="Error: Unsupported category-specific logging level -loglevel=net:abc.",
match=ErrorMatch.PARTIAL_REGEX,
)
self.nodes[0].assert_start_raises_init_error(
extra_args=["-loglevel=net:info:abc"],
expected_msg="Error: Unsupported category-specific logging level -loglevel=net:info:abc.",
match=ErrorMatch.PARTIAL_REGEX,
)
if __name__ == '__main__':
LoggingTest().main()