Skip to content

Commit 789b0bb

Browse files
author
MarcoFalke
committed
Merge #15335: Fix lack of warning of unrecognized section names
1a7ba84 Fix lack of warning of unrecognized section names (Akio Nakamura) Pull request description: In #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
2 parents 849f37f + 1a7ba84 commit 789b0bb

File tree

5 files changed

+36
-24
lines changed

5 files changed

+36
-24
lines changed

src/init.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ void InitParameterInteraction()
839839

840840
// Warn if unrecognized section name are present in the config file.
841841
for (const auto& section : gArgs.GetUnrecognizedSections()) {
842-
InitWarning(strprintf(_("Section [%s] is not recognized."), section));
842+
InitWarning(strprintf("%s:%i " + _("Section [%s] is not recognized."), section.m_file, section.m_line, section.m_name));
843843
}
844844
}
845845

src/test/util_tests.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,10 @@ struct TestArgsManager : public ArgsManager
180180
{
181181
LOCK(cs_args);
182182
m_config_args.clear();
183+
m_config_sections.clear();
183184
}
184185
std::string error;
185-
BOOST_REQUIRE(ReadConfigStream(streamConfig, error));
186+
BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
186187
}
187188
void SetNetworkOnlyArg(const std::string arg)
188189
{

src/util/system.cpp

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -353,23 +353,19 @@ const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
353353
return unsuitables;
354354
}
355355

356-
357-
const std::set<std::string> ArgsManager::GetUnrecognizedSections() const
356+
const std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const
358357
{
359358
// Section names to be recognized in the config file.
360359
static const std::set<std::string> available_sections{
361360
CBaseChainParams::REGTEST,
362361
CBaseChainParams::TESTNET,
363362
CBaseChainParams::MAIN
364363
};
365-
std::set<std::string> diff;
366364

367365
LOCK(cs_args);
368-
std::set_difference(
369-
m_config_sections.begin(), m_config_sections.end(),
370-
available_sections.begin(), available_sections.end(),
371-
std::inserter(diff, diff.end()));
372-
return diff;
366+
std::list<SectionInfo> unrecognized = m_config_sections;
367+
unrecognized.remove_if([](const SectionInfo& appeared){ return available_sections.find(appeared.m_name) != available_sections.end(); });
368+
return unrecognized;
373369
}
374370

375371
void ArgsManager::SelectConfigNetwork(const std::string& network)
@@ -793,7 +789,7 @@ static std::string TrimString(const std::string& str, const std::string& pattern
793789
return str.substr(front, end - front + 1);
794790
}
795791

796-
static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::set<std::string>& sections)
792+
static bool GetConfigOptions(std::istream& stream, const std::string& filepath, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::list<SectionInfo>& sections)
797793
{
798794
std::string str, prefix;
799795
std::string::size_type pos;
@@ -809,7 +805,7 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
809805
if (!str.empty()) {
810806
if (*str.begin() == '[' && *str.rbegin() == ']') {
811807
const std::string section = str.substr(1, str.size() - 2);
812-
sections.insert(section);
808+
sections.emplace_back(SectionInfo{section, filepath, linenr});
813809
prefix = section + '.';
814810
} else if (*str.begin() == '-') {
815811
error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str);
@@ -822,8 +818,8 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
822818
return false;
823819
}
824820
options.emplace_back(name, value);
825-
if ((pos = name.rfind('.')) != std::string::npos) {
826-
sections.insert(name.substr(0, pos));
821+
if ((pos = name.rfind('.')) != std::string::npos && prefix.length() <= pos) {
822+
sections.emplace_back(SectionInfo{name.substr(0, pos), filepath, linenr});
827823
}
828824
} else {
829825
error = strprintf("parse error on line %i: %s", linenr, str);
@@ -838,12 +834,11 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
838834
return true;
839835
}
840836

841-
bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
837+
bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
842838
{
843839
LOCK(cs_args);
844840
std::vector<std::pair<std::string, std::string>> options;
845-
m_config_sections.clear();
846-
if (!GetConfigOptions(stream, error, options, m_config_sections)) {
841+
if (!GetConfigOptions(stream, filepath, error, options, m_config_sections)) {
847842
return false;
848843
}
849844
for (const std::pair<std::string, std::string>& option : options) {
@@ -874,14 +869,15 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
874869
{
875870
LOCK(cs_args);
876871
m_config_args.clear();
872+
m_config_sections.clear();
877873
}
878874

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

882878
// ok to not have a config file
883879
if (stream.good()) {
884-
if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
880+
if (!ReadConfigStream(stream, confPath, error, ignore_invalid_keys)) {
885881
return false;
886882
}
887883
// if there is an -includeconf in the override args, but it is empty, that means the user
@@ -912,7 +908,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
912908
for (const std::string& to_include : includeconf) {
913909
fsbridge::ifstream include_config(GetConfigFile(to_include));
914910
if (include_config.good()) {
915-
if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
911+
if (!ReadConfigStream(include_config, to_include, error, ignore_invalid_keys)) {
916912
return false;
917913
}
918914
LogPrintf("Included configuration file %s\n", to_include.c_str());

src/util/system.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,13 @@ enum class OptionsCategory {
127127
HIDDEN // Always the last option to avoid printing these in the help
128128
};
129129

130+
struct SectionInfo
131+
{
132+
std::string m_name;
133+
std::string m_file;
134+
int m_line;
135+
};
136+
130137
class ArgsManager
131138
{
132139
protected:
@@ -147,9 +154,9 @@ class ArgsManager
147154
std::string m_network GUARDED_BY(cs_args);
148155
std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
149156
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
150-
std::set<std::string> m_config_sections GUARDED_BY(cs_args);
157+
std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
151158

152-
NODISCARD bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
159+
NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
153160

154161
public:
155162
ArgsManager();
@@ -173,7 +180,7 @@ class ArgsManager
173180
/**
174181
* Log warnings for unrecognized section names in the config file.
175182
*/
176-
const std::set<std::string> GetUnrecognizedSections() const;
183+
const std::list<SectionInfo> GetUnrecognizedSections() const;
177184

178185
/**
179186
* Return a vector of strings of the given argument

test/functional/feature_config_args.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,21 @@ def test_config_file_parser(self):
4141
conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass')
4242
self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided')
4343

44+
inc_conf_file2_path = os.path.join(self.nodes[0].datadir, 'include2.conf')
45+
with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
46+
conf.write('includeconf={}\n'.format(inc_conf_file2_path))
47+
4448
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
45-
conf.write('testnot.datadir=1\n[testnet]\n')
49+
conf.write('testnot.datadir=1\n')
50+
with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
51+
conf.write('[testnet]\n')
4652
self.restart_node(0)
47-
self.nodes[0].stop_node(expected_stderr='Warning: Section [testnet] is not recognized.' + os.linesep + 'Warning: Section [testnot] is not recognized.')
53+
self.nodes[0].stop_node(expected_stderr='Warning: ' + inc_conf_file_path + ':1 Section [testnot] is not recognized.' + os.linesep + 'Warning: ' + inc_conf_file2_path + ':1 Section [testnet] is not recognized.')
4854

4955
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
5056
conf.write('') # clear
57+
with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
58+
conf.write('') # clear
5159

5260
def run_test(self):
5361
self.stop_node(0)

0 commit comments

Comments
 (0)