Skip to content

Commit 1a7ba84

Browse files
AkioNakAkioNak
committed
Fix lack of warning of unrecognized section names
1. Fix lack of warning by collecting all section names by moving m_config_sections.clear() to ArgsManager::ReadConfigFiles(). 2. Add info(file name, line number) to warning message. 3. Add a test code to confirm this situation. 3. Do clear() in ReadConfigString().
1 parent 904308d commit 1a7ba84

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
@@ -815,7 +815,7 @@ void InitParameterInteraction()
815815

816816
// Warn if unrecognized section name are present in the config file.
817817
for (const auto& section : gArgs.GetUnrecognizedSections()) {
818-
InitWarning(strprintf(_("Section [%s] is not recognized."), section));
818+
InitWarning(strprintf("%s:%i " + _("Section [%s] is not recognized."), section.m_file, section.m_line, section.m_name));
819819
}
820820
}
821821

‎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
@@ -354,23 +354,19 @@ const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
354354
return unsuitables;
355355
}
356356

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

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

376372
void ArgsManager::SelectConfigNetwork(const std::string& network)
@@ -794,7 +790,7 @@ static std::string TrimString(const std::string& str, const std::string& pattern
794790
return str.substr(front, end - front + 1);
795791
}
796792

797-
static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>>& options, std::set<std::string>& sections)
793+
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)
798794
{
799795
std::string str, prefix;
800796
std::string::size_type pos;
@@ -810,7 +806,7 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
810806
if (!str.empty()) {
811807
if (*str.begin() == '[' && *str.rbegin() == ']') {
812808
const std::string section = str.substr(1, str.size() - 2);
813-
sections.insert(section);
809+
sections.emplace_back(SectionInfo{section, filepath, linenr});
814810
prefix = section + '.';
815811
} else if (*str.begin() == '-') {
816812
error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str);
@@ -823,8 +819,8 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
823819
return false;
824820
}
825821
options.emplace_back(name, value);
826-
if ((pos = name.rfind('.')) != std::string::npos) {
827-
sections.insert(name.substr(0, pos));
822+
if ((pos = name.rfind('.')) != std::string::npos && prefix.length() <= pos) {
823+
sections.emplace_back(SectionInfo{name.substr(0, pos), filepath, linenr});
828824
}
829825
} else {
830826
error = strprintf("parse error on line %i: %s", linenr, str);
@@ -839,12 +835,11 @@ static bool GetConfigOptions(std::istream& stream, std::string& error, std::vect
839835
return true;
840836
}
841837

842-
bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
838+
bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys)
843839
{
844840
LOCK(cs_args);
845841
std::vector<std::pair<std::string, std::string>> options;
846-
m_config_sections.clear();
847-
if (!GetConfigOptions(stream, error, options, m_config_sections)) {
842+
if (!GetConfigOptions(stream, filepath, error, options, m_config_sections)) {
848843
return false;
849844
}
850845
for (const std::pair<std::string, std::string>& option : options) {
@@ -875,14 +870,15 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
875870
{
876871
LOCK(cs_args);
877872
m_config_args.clear();
873+
m_config_sections.clear();
878874
}
879875

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

883879
// ok to not have a config file
884880
if (stream.good()) {
885-
if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
881+
if (!ReadConfigStream(stream, confPath, error, ignore_invalid_keys)) {
886882
return false;
887883
}
888884
// if there is an -includeconf in the override args, but it is empty, that means the user
@@ -913,7 +909,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
913909
for (const std::string& to_include : includeconf) {
914910
fsbridge::ifstream include_config(GetConfigFile(to_include));
915911
if (include_config.good()) {
916-
if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
912+
if (!ReadConfigStream(include_config, to_include, error, ignore_invalid_keys)) {
917913
return false;
918914
}
919915
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
@@ -132,6 +132,13 @@ enum class OptionsCategory {
132132
HIDDEN // Always the last option to avoid printing these in the help
133133
};
134134

135+
struct SectionInfo
136+
{
137+
std::string m_name;
138+
std::string m_file;
139+
int m_line;
140+
};
141+
135142
class ArgsManager
136143
{
137144
protected:
@@ -152,9 +159,9 @@ class ArgsManager
152159
std::string m_network GUARDED_BY(cs_args);
153160
std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
154161
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
155-
std::set<std::string> m_config_sections GUARDED_BY(cs_args);
162+
std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
156163

157-
NODISCARD bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
164+
NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
158165

159166
public:
160167
ArgsManager();
@@ -178,7 +185,7 @@ class ArgsManager
178185
/**
179186
* Log warnings for unrecognized section names in the config file.
180187
*/
181-
const std::set<std::string> GetUnrecognizedSections() const;
188+
const std::list<SectionInfo> GetUnrecognizedSections() const;
182189

183190
/**
184191
* 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)