Repository: bitcoin/bitcoin
File: src/common/config.cpp (C++)

Code before:
                LogPrintf("Ignoring unknown configuration value %s\n", option.first);
            } else {
                error = strprintf("Invalid configuration value %s", option.first);
                return false;
            }
        }
    }
    return true;
}

bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
{
    {
        LOCK(cs_args);
        m_settings.ro_config.clear();
        m_config_sections.clear();
        m_config_path = AbsPathForConfigVal(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME), /*net_specific=*/false);
    }

    const auto conf_path{GetConfigFilePath()};
    std::ifstream stream{conf_path};

    // not ok to have a config file specified that cannot be opened
    if (IsArgSet("-conf") && !stream.good()) {
        error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path));
        return false;
    }
    // ok to not have a config file
    if (stream.good()) {
        if (!ReadConfigStream(stream, fs::PathToString(conf_path), error, ignore_invalid_keys)) {
            return false;
        }
        // `-includeconf` cannot be included in the command line arguments except
        // as `-noincludeconf` (which indicates that no included conf file should be used).
        bool use_conf_file{true};
        {
            LOCK(cs_args);
            if (auto* includes = common::FindKey(m_settings.command_line_options, "includeconf")) {
                // ParseParameters() fails if a non-negated -includeconf is passed on the command-line
                assert(common::SettingsSpan(*includes).last_negated());
                use_conf_file = false;
            }
        }
        if (use_conf_file) {
            std::string chain_id = GetChainTypeString();
            std::vector<std::string> conf_file_names;

            auto add_includes = [&](const std::string& network, size_t skip = 0) {
                size_t num_values = 0;
                LOCK(cs_args);
                if (auto* section = common::FindKey(m_settings.ro_config, network)) {

Review comment (suggestion):
In commit "args: Properly support -noconf and catch directories" (77c8a538b7164f8b65576600eb4668ac160f6b14)

It doesn't seem good to try to open a file with an empty path. This behavior doesn't seem well defined and the call will unnecessarily be passed down to the os when it shouldn't be. I think it would also clarify the code and avoid the need to add a second IsArgNegated call if this checked for an empty path. Would suggest:

```c++
    const auto conf_path{GetConfigFilePath()};
    std::ifstream stream;
    if (!conf_path.empty()) { // path is empty when -noconf is specified
        stream = std::ifstream{conf_path};
        // Calling peek() on ifstreams opened on directories flags them as !good().
        (void)stream.peek();

        // not ok to have a config file specified that cannot be opened
        if (IsArgSet("-conf") && !stream.good()) {
            error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path));
            return false;
        }
    }
```

Code after:
            if (ignore_invalid_keys) {
                LogPrintf("Ignoring unknown configuration value %s\n", option.first);
            } else {
                error = strprintf("Invalid configuration value %s", option.first);
                return false;
            }
        }
    }
    return true;
}

bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
{
    {
        LOCK(cs_args);
        m_settings.ro_config.clear();
        m_config_sections.clear();
        m_config_path = AbsPathForConfigVal(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME), /*net_specific=*/false);
    }

    const auto conf_path{GetConfigFilePath()};
    std::ifstream stream;
    if (!conf_path.empty()) { // path is empty when -noconf is specified
        if (fs::is_directory(conf_path)) {
            error = strprintf("Config file \"%s\" is a directory.", fs::PathToString(conf_path));
            return false;
        }
        stream = std::ifstream{conf_path};
        // If the file is explicitly specified, it must be readable
        if (IsArgSet("-conf") && !stream.good()) {
            error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path));
            return false;
        }
    }
    // ok to not have a config file
    if (stream.good()) {
        if (!ReadConfigStream(stream, fs::PathToString(conf_path), error, ignore_invalid_keys)) {
            return false;
        }
        // `-includeconf` cannot be included in the command line arguments except
        // as `-noincludeconf` (which indicates that no included conf file should be used).
        bool use_conf_file{true};
        {
            LOCK(cs_args);
            if (auto* includes = common::FindKey(m_settings.command_line_options, "includeconf")) {
                // ParseParameters() fails if a non-negated -includeconf is passed on the command-line
                assert(common::SettingsSpan(*includes).last_negated());
                use_conf_file = false;
            }
        }
        if (use_conf_file) {