Skip to content

Commit

Permalink
Merge bitcoin#15335: Fix lack of warning of unrecognized section names
Browse files Browse the repository at this point in the history
1a7ba84 Fix lack of warning of unrecognized section names (Akio Nakamura)

Pull request description:

  In bitcoin#14708, It was introduced that to warn when unrecognized section names are exist in the config file.
  But ```m_config_sections.clear()```  in ```ArgsManager::ReadConfigStream()``` is called every time when reading each configuration file, so it can warn about only last reading file if ```includeconf``` exists.

  This PR fix lack of warning by collecting all section names by moving ```m_config_sections.clear()```  to ```ArgsManager::ReadConfigFiles()``` .
  Also add a test code to confirm this situation.

Tree-SHA512: 26aa0cbe3e4ae2e58cbe73d4492ee5cf465fd4c3e5df2c8ca7e282b627df9e637267af1e3816386b1dc6db2398b31936925ce0e432219fec3a9b3398f01e3e65
  • Loading branch information
MarcoFalke authored and Munkybooty committed Sep 8, 2021
1 parent 3e71e10 commit ef6a5e8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/test/util_tests.cpp
Expand Up @@ -132,9 +132,10 @@ struct TestArgsManager : public ArgsManager
{
LOCK(cs_args);
m_config_args.clear();
m_config_sections.clear();
}
std::string error;
BOOST_REQUIRE(ReadConfigStream(streamConfig, error));
BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
}
void SetNetworkOnlyArg(const std::string arg)
{
Expand Down
9 changes: 5 additions & 4 deletions src/util/system.cpp
Expand Up @@ -927,11 +927,11 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
return true;
}

bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
{
LOCK(cs_args);
std::vector<std::pair<std::string, std::string>> options;
if (!GetConfigOptions(stream, error, options)) {
if (!GetConfigOptions(stream, filepath, error, options)) {
return false;
}
for (const std::pair<std::string, std::string>& option : options) {
Expand Down Expand Up @@ -962,13 +962,14 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
{
LOCK(cs_args);
m_config_args.clear();
m_config_sections.clear();
}

const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
fsbridge::ifstream stream(GetConfigFile(confPath));

if (stream.good()) {
if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
if (!ReadConfigStream(stream, confPath, error, ignore_invalid_keys)) {
return false;
}
// if there is an -includeconf in the override args, but it is empty, that means the user
Expand Down Expand Up @@ -999,7 +1000,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
for (const std::string& to_include : includeconf) {
fsbridge::ifstream include_config(GetConfigFile(to_include));
if (include_config.good()) {
if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
if (!ReadConfigStream(include_config, to_include, error, ignore_invalid_keys)) {
return false;
}
LogPrintf("Included configuration file %s\n", to_include.c_str());
Expand Down
9 changes: 8 additions & 1 deletion src/util/system.h
Expand Up @@ -157,6 +157,13 @@ enum class OptionsCategory {
HIDDEN // Always the last option to avoid printing these in the help
};

struct SectionInfo
{
std::string m_name;
std::string m_file;
int m_line;
};

class ArgsManager
{
protected:
Expand All @@ -178,7 +185,7 @@ class ArgsManager
std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);

[[nodiscard]] bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
[[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);

public:
ArgsManager();
Expand Down
2 changes: 2 additions & 0 deletions test/functional/feature_config_args.py
Expand Up @@ -45,6 +45,8 @@ def test_config_file_parser(self):

with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
conf.write('') # clear
with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
conf.write('') # clear


def test_log_buffer(self):
Expand Down

0 comments on commit ef6a5e8

Please sign in to comment.