C++ Vectors insert new objects

Question

I'm a self-taught c++ programmer (still at novice level).

I think I got an idea of how c++ works, but I can't wrap my head around this: I want to create and populate an std::vector with different elements of a defined-by-me class:

// other code

while (getline(cfgDataStream, cfgData)) //parsing cycle of the config file
{

    std::stringstream ss(cfgData); //creating a stream in order to fill fields
    ss >> string1 >> IP1 >> IP2 >> PORT2 >> INDEX; 
    //they are all strings save the last one, which is a int
    if (ss.fail()) 
    {
        //bad things happen
    }

//FIRST IDEA: Using insert()

    CModbusServer MBtemp* = new CModbusServer(this, IP2.c_str(), PORT2, INDEX)
    std::vector<CModbusServer*>::iterator iterator = this->m_pServerCollection.begin(); //I get the vector initial position

m_pServerCollection.insert(iterator + (INDEX), MBTemp); // I put the new object in the right index (I don't trust the order in the config file)


//SECOND IDEA: Using push_back()

    m_pServerCollection.push_back( new CModbusServer(this, IP2.c_str(), PORT2, INDEX)); //I attach each new object to the end of vector (i trust the order in the config file)
}

basically I want to create an object of CModbusServer and insert its pointer in a vector, so that I have n different CModbusServer objects in each vector position. And this is where I get lost, i tried two ways of insertion (as shown in the code ) without success.

CModbusServer has, among others, a const char* ipAddress field. If I try to access to that field (i.e. to use it in a .Format(_T("%S)) function) I get random data. Trying to see why I noticed that in the vector I don't have n different objects,but n copies of the last object created with new CModbusServer(this, IP2.c_str(), PORT2, INDEX). Probably this happens due to the fact that I have a vector of pointers, but those should be pointers to different objects...

I'm using Visual Studio 2015 with MFC in order to realize a dialog-based application. I have an AppEngine class that calls method from other classes and has a vector of CModbusServer elements. CModbusServer.h is as follows:

class CModbusServer
{
public:    
    CModbusServer(void *parentEngine, const char* , unsigned short , int );
    ~CModbusServer();
    const char* ipAddress;
    unsigned short port;
    int indNode;
    modbus_t *MBserver;
    bool isConnected;
}

So, my question are:

1) Why can't I access to the ipAddress field (instead of reading "192.0.2.1" I read random characters) while I theoretically should be able to read it usingtheApp.CModbusServerVector[properIndex]->ipAddress?

2) I'm making a mistake in populating the vector, but I can't see where and, most importantly, why it's wrong.

Thank you for your assistance, and excuse my english and any omission.

EDIT:

The constructor code of CModbusServer is this:

CModbusServer::CModbusServer(void *pE, const char* ip, unsigned short nport, int ind)
: parentEngine(pE), //used in order to keep track of the parent dialog
ipAddress(ip),
port(nport),
indNode(ind)
{
this->isConnected = false;
this->m_socket = INVALID_SOCKET;

memset(&m_socketstructhint, 0, sizeof m_socketstructhint);
m_socketstructhint.ai_family = AF_UNSPEC;
m_socketstructhint.ai_socktype = SOCK_STREAM;
m_socketstructhint.ai_protocol = IPPROTO_TCP;

MBserver = modbus_new_tcp(ipAddress, (int)nport);


}

Please tell me if I omitted any other useful information.

Initially I used CString for managing the strings, but then I incurred in more and more issues and finally got a compiling and patially working code with const char*. I managed to enstablish a connection and to read the desired modbus registers, but then I got stuck on the isAddress printing problem.

modbus_new_tc(ip,port) is a method found in libmodbus library, a freeware library written for C that I had to use.

EDIT 2: related to angew answer:

So, If I'm right, what's happening is that I create a temporary set of pointers, that are used by the constructor (I've now added the relevant code). But shouldn't be the constructed object unrelated with what I've passed as argument? Aren't those argument copied? Sorry if the question is stupid, but I'm still learning.

the indices are sequential, though in the config file thay could as well be 0-1-2-3 (1 per line) or 0-3-1-2, that's what I meant by "don't trust them".

Since the push_back method has the same issues, probably the problem is back in the constructor. What puzzles me is that by doing a step by step execution I can see that with each iteration of the while loop I get new and correct data, but instad of being put in the i-th position, is put in the first i positions (i.e.: origin data: a b c, 1st run vector = a; 2nd run vector = b b,3rd run vector = c c c)

I didn't know std::unique_ptr<>, i'll look it up.

I've tried to use std:string or even CString, but the problem lies beneath libmodbus libraries.

Answer 1

Calling c_str on a std::string returns a "live" pointer to the internal data stored in that std::string instance. The pointer returned points to a buffer which is only valid as long as the std::string on which it was called remains alive and unmodified.

The constructor of CExtracalModbusServer just stores the pointer passed in. That pointer becomes dangling as soon as IP2 is re-assigned in the next iteration of the input loop. (The address pointed to is still the same, but the buffer which was previously located at that address has either been overwritten, or freed, or something else. In other words, the pointer is just dangling).

As for inserting into the vector: the first way (with insert) could only work if the indices in the files are sequential and starting at 0. You need a valid iterator into the vector to insert into it, where valid here means either pointing to one of the elements already in the vector, or the past-the-end iterator (the one returned by end()). If INDEX is equal to or larger than the size of the vector, then m_pServerCollection.insert(iterator + (INDEX), MBTemp); will attempt to insert outside the vector (essentially a buffer overfow). Ubnefined behaviour ensues.

The push_back way of inserting data should work, and if you see it misbehaving, it's either an artifact of the earlier bug (the one with dangling pointers), or there is a separate problem in code which you haven't shown.

---

Unrelated to the question at hand, but the code contains pretty bad practice in the form of manually managed dynamic memory. Instead of storing a CModbusServer* in the vector and managing the memory manually with delete in all the right places, you should be using std::unique_ptr<CModbusServer> which will take care of proper deallocation for you, even in the presence of exceptions.

If CModbusServer is under your control, you should likewise change it to store a std::string instead of a const char*. Never use C-style strings in C++ unless you have to interact with C-style APIs, and even in such case, limit their use only to the interaction itself. It's the same principle all over again: don't manage memory manually.