Segfault while iterating through std::list<USER_DEFINE_CLASS> with iterator

Question

I am facing a problem in my program where when I try to iterate through my std::list<User_Defined_Class> with iterator, I get a segmentation fault when I try to access the second iterator.

Here is the full program, first I instantiate 3 servers and set to them random port number for debugging purpose, then I push them into std::list<Server> private member of tcp_servers with the help of the public method pushNewServer who is implemented like so:

void TcpServer::pushNewServer(const Server& server)
{
    _servers.push_back(server);
};

and finally I create an Iterator of the same as my list and iterate until I face the end of my list however when I launch this program I get a segfault.

#include <string>
#include <iostream>
#include <list>

# define GET 1
# define POST 2
# define DELETE 3
# define SET -1
# define CLEAR -2
# define ALL_METHODS 7

#define bitset(byte,nbit)   (byte |= (1 << nbit))
#define bitclear(byte,nbit) (byte &= ~(1 << nbit))
#define bitcheck(byte,nbit) (byte & (1 << nbit))

class Server;
class Location;
class TcpServer
{
    public:
        TcpServer(const std::string& filename){(void)filename;};
        ~TcpServer(){};

        /*GETTERS*/
        std::list<Server> getServers(void) const {return _servers;};

        /*SETTERS*/
        void pushNewServer(const Server& server){_servers.push_back(server);};
    private:
        TcpServer(const TcpServer& rhs);
        TcpServer& operator=(const TcpServer& rhs);
        TcpServer();
        std::list<Server> _servers;
        /* data */
};




class Location
{
    public:
        Location()
            :_location_options(0),_body_size(0),_index(""),_root_dir(""),_uri(""),_server(0){};
        Location(const Location& rhs)
        {
            _location_options = rhs._location_options;
            _body_size = rhs._body_size;
            _root_dir = rhs._root_dir;
            _index = rhs._index;
            _uri = rhs._uri;
            _sub_locations = rhs._sub_locations;
            _server = rhs._server;
        }
        Location& operator=(const Location& rhs)
        {
            if (this == &rhs) return *this;
            _location_options = rhs._location_options;
            _body_size = rhs._body_size;
            _root_dir = rhs._root_dir;
            _index = rhs._index;
            _uri = rhs._uri;
            _sub_locations = rhs._sub_locations;
            _server = rhs._server;
            return *this;
        }
        ~Location(){};
    
        /*GETTERS*/
        unsigned int&  getLocationsOptions(void) {return _location_options;};
        const unsigned int& getBodySize(void) const {return _body_size;};
        const std::string& getRootDir(void) const {return _root_dir;};
        const std::string& getIndex(void) const {return _index;};
        const std::string& getUri(void) const {return _uri;};
        std::list<Location> getSubLocations(void) const {return _sub_locations;};
        const Server* getServer(void) const {return _server; };

        /*Setters*/
        void    setBodySize(const unsigned int& body)
        {
            _body_size = body;
        };
        void    setIndex(const std::string& index)
        {
            _index = index;
        };
        void    setRootDir(const std::string& root_dir)
        {
            _root_dir = root_dir;
        };
        void    setUri(const std::string& uri)
        {
            _uri = uri;
        };
        void    pushNewLocation(const Location& location)
        {
            _sub_locations.push_back(location);
        };
        void    setServer(Server *server)
        {
            _server = server;
        };

        void    setLocationOption(const unsigned int& nbit, char actions)
        {
            if (actions == SET)
                bitset(_location_options, nbit);
            else if (actions == CLEAR)
                bitclear(_location_options, nbit);
        };

        /*MEMBER FUNCTION*/
        bool    checkBits(const unsigned int& nbit) const
        {
            return (bitcheck(_location_options, nbit) > 0);
        }
    private:
        unsigned int    _location_options;
        unsigned int    _body_size;
        std::string _index;
        std::string _root_dir;
        std::string _uri;
        std::list<Location> _sub_locations;
        Server  *_server;
};


class Server
{
    public:
        Server()
            :_serv_options(0),_port(0),_body_size(0),_root_dir(""),_index(""){};
        Server(const Server& rhs)
        {
            _serv_options = rhs._serv_options;
            _body_size = rhs._body_size;
            _port = rhs._port;
            _root_dir = rhs._root_dir;
            _index = rhs._index;
            _server_names = rhs._server_names;
            _locations = rhs._locations;
        }
        Server& operator=(const Server& rhs)
        {
            if (this == &rhs) return *this;
            _serv_options = rhs._serv_options;
            _body_size = rhs._body_size;
            _port = rhs._port;
            _root_dir = rhs._root_dir;
            _index = rhs._index;
            _server_names = rhs._server_names;
            _locations = rhs._locations;
            return *this;
        }
        ~Server(){};

        /*GETTERS*/
        unsigned int  getServOptions(void) const {return _serv_options;};
        const unsigned int& getPort(void) const {return _port;};
        const unsigned int& getBodySize(void) const {return _body_size;};
        const std::string& getRootDir(void) const {return _root_dir;};
        const std::string& getIndex(void) const {return _index;};

        std::list<Location> getLocations(void) const {return _locations;};
        std::list<std::string> getServerNames(void) const {return _server_names;};

        /*Setters*/
        void    setPort(const unsigned int& port)
        {
            _port = port;
        };
        void    setBodySize(const unsigned int& body)
        {
            _body_size = body;
        };
        void    setRootDir(const std::string& root_dir)
        {
            _root_dir = root_dir;
        };
        void    setIndex(const std::string& index)
        {
            _index = index;
        };
        void    pushNewServerName(const std::string& server_name)
        {
            _server_names.push_back(server_name);
        };
        void    pushNewLocation(const Location& location)
        {
            _locations.push_back(location);
        };
        void    setServOption(const unsigned int& nbit, char actions)
        {
            if (actions == SET)
                bitset(_serv_options, nbit);
            else if (actions == CLEAR)
                bitclear(_serv_options, nbit);
        };

        /*MEMBER FUNCTION*/
        bool    checkBits(const unsigned int& nbit) const
        {
            return (bitcheck(_serv_options, nbit) > 0);
        }
    private:
        unsigned int    _serv_options;
        unsigned int    _port;
        unsigned int    _body_size;
        std::string     _root_dir;
        std::string     _index;
        std::list<std::string> _server_names;
        std::list<Location> _locations;
};



int main (int argc, char **argv)
{
    try
    {
        TcpServer tcp_servers(argc > 1 ? argv[1] : "");
        Server a;
        Server b;
        Server c;
        a.setPort(1500);
        b.setPort(2500);
        c.setPort(3500);
        tcp_servers.pushNewServer(a);
        tcp_servers.pushNewServer(b);
        tcp_servers.pushNewServer(c);
        std::list<Server>::iterator it;
        for (it = tcp_servers.getServers().begin(); it != tcp_servers.getServers().end() ; it++)
        {
            std::cout << &*it << std::endl;
            std::cout << "-----------NEXT_SERV-------------\n\n";
        }
    }
    catch(const std::exception& e)
    {
        std::cerr << e.what() << std::endl;
    }
    
}

Answer 1

The problem is with the TcpServer::getServers function:

std::list<Server> getServers(void) const {return _servers;};

Because it returns the list by value, the loop

for (it = tcp_servers.getServers().begin(); it != tcp_servers.getServers().end() ; it++)

will have two different lists that it compares the iterators with.

The iterator you get from tcp_servers.getServers().begin() will be for a totally different list than the iterator you get from tcp_servers.getServers().end().

To be able to compare iterators they must both come from the exact same container.

The simple solution is to return the list by reference instead:

std::list<Server>& getServers(void) const {return _servers;};

---

On a different note, the default container should really be std::vector. Unless you have very specific requirements (and I don't see any here) there no need for std::list.

Also please don't use explicit iterator for loops unless you have very specific requirements for it. A range for loop would work just as well:

for (auto& server : tcp_servers.getServers())
{
    std::cout << &server << '\n';
}