Strange delete behavior

[jessemelhuish]

I have to make a class that manages a pile of cards. It dynamically expands the array that is created with a pointer. Because I'm using a pointer I have a destructor in my class, but whenever it runs it outputs correctly but then give a segmentation fault. I used gdb to trace the fault back to this line:

delete [] pile;

Everywhere I check says that is acceptable structure for that delete. Have I messed something up that is plainly obvious? Thanks in advance!

[helios]

How is pile declared and initialized?

[jessemelhuish]

declaration:

MysteryCard * pile;

The constructor sets it to null first:

pile = NULL; And here is the function that expands it:


void SortedPile::expand_pile(int increase)
{
MysteryCard * temppile = new MysteryCard[pileSize + increase];

//x is a counter used to go through the loop to copy all of the
//values from one set of memory to another.
for (int x = 0;x < pileSize;x++)
{
temppile[x] = pile[x];
}

//Here the pile size is increasedto reflect the expansion, the old
//memory is deallocated, pile is pointed to the new memory, and
//temppile is set to null.
pileSize += increase;
delete [] pile;
pile = temppile;
temppile = NULL;
}

[shacktar]

That looks OK. The error is probably somewhere else in the code. Is pileSize set to zero in the constructor? Does MysteryCard need/have an overloaded assignment operator/destructor?

[jessemelhuish]

Yes, pileSize is set to zero in the constructor. Yes it has one, and it does copy correctly(just checked). The error seems to be happening right at that line, at least that's what gdb is saying. I can post the entire thing if you'd like.

[shacktar]

I can post the entire thing if you'd like.

Alright, go ahead (can't hurt)

[jessemelhuish]

#include "SortedPile.h"

//SortedPile initializes the pointer by settign it equal to null, and
//then setting the size to zero.
SortedPile::SortedPile()
{
  pile = NULL;
  pileSize = 0;
}

//~SortedPile checks to see if the pointer is pointing to null.  If it
//is not then it deletes it and points it to null.
SortedPile::~SortedPile()
{
  if (pile != NULL)
    {
      delete[] pile;
      pile = NULL;
    }
}

//add_card first checks to see if the given card is already present in
//the deck, if it isn't, then it expands the deck by one and inserts
//the card.  If the card is already present it will return false,
//otherwise it will return true.
bool SortedPile::add_card(MysteryCard & card)
{
  //Here is the check for duplicates
  if(is_double(card))
    {
      //cout, not cerr because this is an expected possible outcome.
      cout << "Card is a duplicate\n";
      return false;
    }

  expand_pile(1);
  insert_card(card);

  return true;
}

//This is the function that is used to read in a pile of cards from a
//file.  It asks the user for a name of a file and attempts to open
//it.  If it can't be opened ann error is output and the function
//terminates with false.  After that, the function enters into a loop,
//reading in the cards until it reaches the end of the file.  If the file is read succesfully, true will be returned.
bool SortedPile::read_pile()
{
  string filename;
  cout << "Please enter the name of the file that contains the pile.\n";
  cin >> filename;

  ifstream inFile(filename.c_str());

  if (inFile.fail())
    {
      cerr << "There was a problem opening the file." << endl;
      return false;
    }

  MysteryCard temp;
  int valtemp;

  //For each iteration the loop will check to make sure the file has
  //not ended, then it will expand the pile and insert the card.
  while (!inFile.eof())
    {
      expand_pile(1);
      inFile >> valtemp;
      temp.change(valtemp);
      insert_card(temp);
    }

  return true;
}

//This function is used to determine if the card already exists in the
//pile.  It loops through the pile and checks card against all of the
//cards in the pile, returning true if it finds a match.
bool SortedPile::is_double(MysteryCard & card)
{
  //x is a counter that is used to search throught the pile.
  for (int x = 0;x < pileSize;x++)
    {
      if (pile[x] == card)
	{
	  return true;
	}
    }

  return false;
}

//This function will perform the steps necessary to insert a card into
//the pile. NOTE: the calling function should expand the file first,
//as this funciton will assume that it has been expanded.
void SortedPile::insert_card(MysteryCard & card)
{
  pile[pileSize - 1] = card;
  sort_pile();
}

//This function is used to expand the pile so that another card(s) can
//be added.  when called it creates a new pointer of greater size, and
//then copies the values form the old array into the new, leaving
//extra space for another card.
void SortedPile::expand_pile(int increase)
{
  MysteryCard * temppile = new MysteryCard[pileSize + increase];

  //x is a counter used to go through the loop to copy all of the
  //values from one set of memory to another.
  for (int x = 0;x < pileSize;x++)
    {
      temppile[x] = pile[x];
    }

  //Here the pile size is increasedto reflect the expansion, the old
  //memory is deallocated, pile is pointed to the new memory, and
  //temppile is set to null.
  delete[] pile;
  pileSize += increase;
  pile = temppile;
  temppile = NULL;
}

//This function is used to sort the pile after a new card has been
//added.  It uses a basic bubble sort to sort the pile.
void SortedPile::sort_pile()
{
  MysteryCard temp;

  //x is used to keep the count of how many times we will go through
  //the array, each time i is used for determining where the current
  //comparison is.
  for (int x = 0;x < pileSize;x++)
    {
      for(int i = 0;i < (pileSize - x);i++)
	{
	  if (pile[i] > pile[i + 1])
	    {
	      temp = pile[i];
	      pile[i] = pile[i + 1];
	      pile[i + 1] = temp;
	    }
	}
    }
}

[Nisheeth]

Please use [code][/code] to format your code. You should edit the previous post and for-mat the code to make it readable...

[jessemelhuish]

Sorry about the code, I forgot that it wouldn't format it.

[firedraco]

In your destructor you don't need to check for NULL, delete does nothing if the pointer is NULL.

Other then that...I don't see anything wrong. Hrm. Only thing I can think of is that maybe you are going out of bounds on your array somewhere...

[shacktar]

Yeah, I guess we'll have to see the MysteryCard code as well as the code that uses SortedPile :s

EDIT: Scratch that

[gpotw]

It looks like you're going out of bounds in sort_pile(). In the first iteration of x, i goes up to (pileSize-1), the upper boundary of the pile array. So i+1 is out of bounds:

pile[i + 1] = temp;