Array setup in constructor means failure later on

Question

I had an issue where my code segfaulted on attempting to use the size() function of a list. On the advice of stackoverflow :-) I constructed a minimum case in which the segfault occurs (on the call inventory.size() below). It is:

#include <list>

class Thing {};

class Player {
private:
  int xpCalcArray[99];
  std::list<Thing*> inventory;

public:
  Player();

  int addToInv(Thing& t); // return 1 on success, 0 on failure
};

Player::Player() {
  // set up XP calculation array
  for (int i=1; i<100; i++) {
    if (i<=10) {
      xpCalcArray[i] = i*100;
    }
    if (i>10 && i<=50) {
      xpCalcArray[i] = i*1000;
    }
    if (i>50 && i<=99) {
      xpCalcArray[i] = i*5000;
    }
  }
}

int Player::addToInv(Thing& t) {
  if (inventory.size() == 52) {
  return 0;
  } else {
      inventory.push_back(&t);
  }
  return 1;
}

int main(int argc, char *argv[]) {
  Thing t;
  Player pc;
  pc.addToInv(t);
  return 1;
}

I notice that when I remove the setting up of the array in the Player cosntructor, it works fine, so this looks to be the problem. What am I doing wrong?

Answer 1

You are accessing your array out of bounds, which results in undefined behaviour. The valid index range for this array

int xpCalcArray[99];

is 0 to 98. You are accessing index 99 here:

if (i>50 && i<=99) {
  xpCalcArray[i] = i*5000;
}

Your outer loop should be

for (int i=0; i<99; i++) { ... }

Note I start from 0, although it is an assumption that you actually want to access the first element.

Then your final condition can be simplified to

if (i>50) {
  xpCalcArray[i] = i*5000;
}

If you intended to use a size 100 array, then you need

int xpCalcArray[100];

then loop between int i=0; i<100;.

Answer 2

You are accessing outside the bounds of your array. Doing so causes undefined behaviour and so there is no logical explanation for anything that occurs afterwards. The size of your array is 99 and so the last index is 98. Your for loop goes up to 99, however.

Either make your array size 100:

int xpCalcArray[100];

Or change your for condition to i < 99.

Answer 3

You are overwriting your array of 99 ints by attempting to modify the 2nd→100th elements (rather than 1st→99th).

In your case, this happens to overwrite some memory within the std::list<Thing*> (which exists in memory directly after the array — not always, but evidently for you today) and thus, when you try to use the list, all hell breaks loose when its internal member data is no longer what it thought it was.