std::vector overwriting final value, rather than growing?

Question

I'm having an issue where using vector.push_back(value) is overwriting the final value, rather than appending to the end. Why might this happen? I have a sample item in the vector, so it's size never hits zero. Below is the code..

void UpdateTable(vector<MyStruct> *Individuals, MyStruct entry)
{
    MyStruct someEntry;
    bool isNewEntry = true;

    for (int i = 0; i < Individuals->size(); i++)
    {
        if (!(strcmp(Individuals->at(i).sourceAddress, entry.sourceAddress)))
        {
            isNewEntry = false;
            //snip.  some work done here.
        }
    }

    if(isNewEntry)
    {
        Individuals->push_back(entry);
    }
}

This let's my first "sample" value stay in, and will allow for just one more item in the vector. When 2 new entries are added, the second overwrites the first, so the size is never larger than 2.

edit: More code, since this is apparently not the issue?

void *TableManagement(void *arg)
{
      //NDP table to store discovered devices.
      //Filled with a row of sample data.
      vector<MyStruct> discoveryTable;
      MyStruct sample;
      sample.sourceAddress = "Sample";
      sample.lastSeen = -1;
      sample.beaconReceived = 1;
      discoveryTable.push_back(sample);

      srand(time(NULL));
      while(1)
      {
          int sleepTime = rand() % 3;
          sleep(sleepTime);
          MyStruct newDiscovery = ReceivedValue();
          if (newDiscovery.lastSeen != -1000) //no new value from receivedValue()
          {
              UpdateTable(&discoveryTable, newDiscovery);
          }
          printTable(&discoveryTable);
      }
      return NULL;
}

Answer 1

Your code looks alright to me. Is it possible that you are not passing the right vector in? What I mean is that the behaviour you describe would appear if somehow your Individuals vector is being reset to its orginal 1-entry state before you tried to add the 3rd entry, then it would appear as if your 2nd entry was being overwritten.

Here is what I mean:

int test_table()
{
  string SampleAddresses[] = {"Sample Address 1", "Sample Address 2"};

  for (int i = 0; i < 2; i++)
  {
     // All this work to build the table *should* be done outside the loop; but we've accidentally put it inside
     // So the 2nd time around we will destroy all the work we did the 1st time
     vector<MyStruct> Individuals;
     MyStruct Sample;
     Sample.sourceAddress = "Sample Address 0";
     Test.push_back(Sample);

     // this is all we meant to have in the loop
     MyStruct NewEntry;
     NewEntry.sourceAddress = SampleAddresses[i];
     UpdateTable(Individuals, NewEntry);
  }

  //Now the table has 2 entries - Sample Address 0 and Sample Address 2.
}

If this was all your code then the problem would be obvious. But it might be concealed in some other pieces of code.

Answer 2

Seems odd to me: Maybe there is something wrong with the //snip part of the code?

Try to log the size of the vector before and after the push_back call (either in the debugger or using cout) and also have a look at the isNewEntry variable.

Answer 3

The scope for the items you are pushing into the database is expiring. They're being destructed when you leave the {} in which they were created - and as such the reference to them is no longer valid.

You need to change it from vector<MyStruct> to vector<MyStruct*> (preferably using safe pointers from Boost:: instead of pointers, but you get the idea).

You are creating the item within the (limited) scope and pushing it onto the vector (while the struct is copied, the strings in it are not!) it then reuses the same memory location (most likely if properly optimized) to store the next "new" struct and the one after that and so on and so forth.

Instead, within the limited scope create MyStruct *myObject = new MyStruct and assign its values, then push the pointer to the vector.

Remember to delete all values from the vector before clearing it/destroying it!!

Or, of course, you could use std::string/CString/whatever instead of a char array and avoid the issue entirely by having a safe-to-copy struct.

Answer 4

ComputerGuru's answer works however there in another alternative. You can create a copy constructor and overload operator= for MyStruct. In these operations, you need to copy the actual string into the new struct. In C++, structs are nothing more than classes with default public access instead of default private access. Another alternative is to use std::string instead of char* for the string value. C++ strings already have this behavior.

struct MyStruct {
   std::string sourceAddress;
   int lastSeen;
   int beaconReceived;
};

Answer 5

I'm going to hazard a guess:

Suppose MyStruct is declared like

struct MyStruct
{
    const char *sourceAddress;
    // Other Gubbins ...
};

And that ReceivedValue does something like

MyStruct ReceivedValue()
{
    static char nameBuffer[MAX_NAME_LEN];

    // Do some work to get the value, put the name in the buffer

    MyStruct s;
    s.sourceAddress = nameBuffer;
    // Fill out the rest of MyStruct
    return s;
}

Now, every structure you push into your vector has sourceAddress pointing to the same global buffer, every time you call ReceivedValue it overwrites that buffer with the new string - so every entry in your vector ends up with the same string.

I can't be sure without seeing the rest of your code, but I can be sure that if you follow some of the good C++ style suggestions in the comments to your question this possiblity would go away.

Edit for clarification: there's no need to heap allocate your structures, simply declaring sourceAddress as a std::string would be sufficient to eliminate this possibility.