-2

So I've declared an array of pointers that look something like

Items* _items[ARRSIZE];

Basically with the goal of using them as an array of objects (one for meat, one for produce) which is dynamically decided on run time. I'm calling the following function in my constructor and I've identified it as the reason I keep segfaulting before the main function.

void Inventory::loadRecs(){
    datafile.open(_filename);
    int i = 0;

    char c;
    //create fileif it doesnt exist
    if(datafile.fail()){
        datafile.clear();
        datafile.close();
        datafile.open(_filename, ios::out);
        datafile.close();
    }
    else{
       //read from file
       while(!datafile.eof()){
           if(_items[i] != nullptr){
               delete _items[i];
           }
           c = datafile.get();
           if(c == 'P'){
               _items[i] = new Produce;
           }
           if (c == 'M'){
               _items[i] = new Meat;
           }
           datafile.ignore();
           _items[i]->load(datafile);

           i++;
           datafile.ignore(); //ignore endl
       }
       _noOfItems = i;
       datafile.close();
   }
} 

The text file I'm reading from is fairly straight forward reading something like

P,123,carrots,0.66,[NEWLINE] The first character identifies what kind of product it is (meat or produce) and the rest of the line is read in with the load function.

My Inventory class looks something like this:

 class Inventory{
    char _filename[256];
    Item* _items[5];
    std::fstream datafile;
    int _noOfItems;
}

And the constructor just initializes everything and calls loadsRecs (which is where I get my segfault from)

b4hand
  • 8,601
  • 3
  • 42
  • 48
user3551329
  • 3
  • 1
  • 1
  • 6
  • How does `Item::load()` looks like? – timrau Apr 29 '15 at 02:31
  • [Don't use datafile.eof(), it could have something to do with your problem.](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – dwcanillas Apr 29 '15 at 02:42
  • Also it's better to use RAII rather than explicit calls to `open` and `close`. – b4hand Apr 29 '15 at 02:43
  • It's also a bad practice to use leading underscores for member variables as you can get conflicts with [reserved identifiers](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). – b4hand Apr 29 '15 at 02:47
  • 1
    Is there any reason you can't use Standard Library containers? Seeing static sized arrays like `char _filename[256]` is almost always a sign you've got a serious error in your program from a lack of bounds checking. `std::string` does not have these problems. – tadman Apr 29 '15 at 15:46

1 Answers1

0

I'm willing to bet that you aren't initializing your array of pointers and so your check against nullptr is failing, and you are calling delete on a garbage pointer resulting in undefined behavior.

Unless you have a constructor that you omitted from your code, the pointers are default initialized resulting in indeterminate values.

Since in the code you have shown, you are using new Produce; and new Meat;, I'll assume you have written new Inventory; or Inventory i;. If instead, you included the parenthesis (or braces in C++11) like new Produce(); or Inventory i{};, you would get value initialization which would do zero initialization of your pointers. This would result in the behavior that you seemingly expect.

b4hand
  • 8,601
  • 3
  • 42
  • 48
  • wouldn't Items _items[5]; initialize them? – user3551329 Apr 29 '15 at 02:51
  • 1
    In C++11, [default initialization](http://en.cppreference.com/w/cpp/language/default_initialization) of arrays does default initialization of the values. In this case, your values are pointers *not* objects. Thus they are initialized to indeterminate values assuming that your `Inventory` object is default initialized. – b4hand Apr 29 '15 at 15:39