CODEWITHSUNDEEP
c++delete-operator
Jed Stevens
Jed Stevens

Reputation: 19

Reallocation of Array of Pointers in C++, using new and delete only

Carray.h

#ifndef CARRAY_H
#define CARRAY_H

#include "Cell.h"
#include "Pirate.h"

class CArray
{
  public:
    CArray();
    ~CArray();
    int   getSize();
    Cell* get(int);
    int   add(Cell*);
  private:
    int    size;
    Cell** elements;
};

#endif

CArray.cc

CArray::CArray() : size(0)
{ 
  elements = new Cell*[size];
}

CArray::~CArray() 
{
  for (int i=0; i<size; ++i)
    delete elements[i];
}

int CArray::add(Cell* cell)
{
  Cell** tmp = new Cell*[size];
  tmp = elements;
  elements = new Cell*[size+1];

  for (int i = 0; i < size; i++){
    *(elements + i) = *(tmp + i);
  }

  *(elements + size) = cell;

  delete [] tmp;

  size++;
}

This last function gives a memory leak and I'm not sure as to where. All I know is that each new must be matched with a delete and each new[] with a delete[]. However testing this in valgrind and trying to delete[] elements before I reallocate elements gives me a Segmentation Fault. If someone could explain where I'm going wrong I would be a happy camper. I'm sorry if this is a duplicate, but I couldn't find anything specific enough to solve my problem.

Upvotes: 0

Views: 310

Answers (2)

PaulMcKenzie
PaulMcKenzie

Reputation: 35455

You are allocating for tmp and then immediately losing the pointer returned to you by assigning to tmp:

Cell** tmp = new Cell*[size];
tmp = elements;

The lines above produce a memory leak, since the value returned by new is lost as soon as the second line is executed.

All I know is that each new must be matched with a delete and each new[] with a delete[].

That is not the only thing to consider. The value returned by new/new[] must be the same value you use when calling delete/delete[].

To fix your function, you should do this:

int CArray::add(Cell* cell)
{
  Cell** tmp = new Cell*[size + 1];

  for (int i = 0; i < size; i++)
    tmp[i] = elements[i];  
  tmp[size] = cell;

  ++size;
  delete [] elements;
  elements = tmp;
  return 1;
}

Note how the elements are copied to the tmp array, then it's just a matter of simply getting rid of the old memory (which is elements) and assign to elements the new data.

Also, there is another subtle bug in that you have a return value of int, but you didn't return anything. Not returning a value from a function that says it returns a value is undefined behavior.

Upvotes: 2

amchacon
amchacon

Reputation: 1961

In the two first lines

Cell** tmp = new Cell*[size];
tmp = elements;

You allocate memory, take direction of new in tmp and replace pointer. So, you lost the direction of new.

Upvotes: 1

Related Questions