ini file parser in C++

Please review my ini file parser (and potentially some general config file formats).

The data structure used is a section, see below. Does that seem a good approach?

I was wondering about the name. It will parse ini files ok, but unix config files are so variable that calling the class config might be over selling it a bit.

I don't support mid-line comments. Only comments on a line on their own are well just ignored.

All values are stored as strings. I thought that only the user of the config file will know if a value should be used in his or her application as a string or integer or whatever and onus is on user to convert.

Any comments would be much appreciated. Its intended initial use is for Windows ini files. Parsing the configuration into sections.

config.hpp

#ifndef CONFIG_HPP_
#define CONFIG_HPP_

#include 
#include 
#include 

struct section
{
    std::string name;
    std::unordered_map keyvalues;
};

class config
{
public:
    config(const std::string& filename);

    section* get_section(const std::string& sectionname);
    std::list& get_sections();

    std::string get_value(const std::string& sectionname, const std::string& keyname);

private:
    void parse(const std::string& filename);

    std::list sections;
};

#endif // CONFIG_HPP_

config implementation - config.cpp

```
#include "config.hpp"

#include
#include
#include
#include
#include
#include

// trim leading white-spaces
static std::string& ltrim(std::string& s) {
size_t startpos = s.find_first_not_of(" \t\r\n\v\f");
if (std::string::npos != startpos)
{
s = s.substr(startpos);
}
return s;
}

// trim trailing white-spaces
static std::string& rtrim(std::string& s) {
size_t endpos = s.find_last_not_of(" \t\r\n\v\f");
if (std::string::npos != endpos)
{
s = s.substr(0, endpos + 1);
}
return s;
}

config::config(const std::string& filename) {
parse(filename);
}

section* config::get_section(const std::string& s

I see some things that may help you improve your program.

Use all of the required #includes

The function strncpy is used but its declaration is in #include which is not actually in the list of includes.

Don't expose class internals

The sections member data is a private data member, which is fine and appropriate, but then the get_sections() returns an unfettered reference to it, rendering the private classification nearly meaningless. The way it's currently written, one can arbitrarily modify all internal data via that one function which is not good object-oriented design. Better would be to either return a copy or at least to return a const reference.

Prefer std::istream to file names

The current interface gets passed a file name, but that's a relatively inflexible and unsatisfactory solution. For one thing, as your parse code comment notes, if there's a problem opening a file (or doing anything else with it) one would normally expect that either the code handles it or that it returns some indication of error. This code does neither. Secondly, as the test code illustrates, it would have been nice if the interface had been able to accomodate a std::istream instead of a filename. This would have allowed usage like this:

std::stringstream ss{"[protocol]\nversion = 6     \n\n[user]\nname = Bob Smith       \n"
    "email = bob@smith.com \nactive = true\n\npi = 3.14159"};
config cfg(ss);

Use auto to simplify your code

Within the get_value member function we have this line:

std::unordered_map::const_iterator it = sect->keyvalues.find(keyname);

This could be made much simpler by the use of auto:

const auto it = sect->keyvalues.find(keyname);

Use const where practical

Several of your member functions are lookup functions which do not and should not alter the underlying config object. Those functions, such as get_value, should be declared const:

std::string get_value(const std::string& sectionname, const std::string& keyname) const;

Reconsider the data structures

The basic usage of the config object is to be able to fetch individual items by section and key name. The interface right now looks like this when used:

std::cout << "email=" << cfg.get_value("user", "email") << '\n';

For this reason, it seems to me that it would make more sense to use a different data structure than the current std::list. Instead, one could use a std::unordered_map of std::unordered_maps. That would make a number of things a bit simpler.

Support write operations

It may be useful to allow the user to modify settings and then write the updated configuration file back out, but the current interface allows no way to do this.

Add better error handling

There are various comments within the code indicating an intention of adding error handling or at least error signalling. This would be useful, but there are other things that might go wrong that should be handled, such as an incomplete section name at the end of a file or items not in any named section.

Consider the use of regular expressions

Many of the things in this code would be much smaller and more concise if they were written using the C++11 regex library. For one thing, the ltrim and rtrim functions would become obsolete. As an example, here's what the parse function looks like with regex and also the data structure change I mentioned (I renamed the private variable map):

void config::parse(std::istream& in) {
    static const std::regex comment_regex{R"x(\s*[;#])x"};
    static const std::regex section_regex{R"x(\s*\[([^\]]+)\])x"};
    static const std::regex value_regex{R"x(\s*(\S[^ \t=]*)\s*=\s*((\s?\S+)+)\s*$)x"};
    std::string current_section;
    std::smatch pieces;
    for (std::string line; std::getline(in, line);)
    {
        if (line.empty() || std::regex_match(line, pieces, comment_regex)) {
            // skip comment lines and blank lines                    
        }
        else if (std::regex_match(line, pieces, section_regex)) {
            if (pieces.size() == 2) { // exactly one match
                current_section = pieces[1].str();
            }
        }
        else if (std::regex_match(line, pieces, value_regex)) {
            if (pieces.size() == 4) { // exactly enough matches
                map[current_section][pieces[1].str()] = pieces[2].str();
            }
        }
    }
}

StackExchange Code Review Q#127819, answer score: 12