c++ Error in free when using array as parameter

Question

I've read many posts with the same error, unfortunately all of those deal with indexing off the end of an array. In my case I get the error when I assign the array to a variable in my constructor.

Here is my code:

Heap.cpp

#include "./Heap.h"
#include <iostream>
#include <sstream>
// Provides floor, ceil, etc.
#include <cmath>

using namespace std;

Heap::Heap() {
  arraySize = 0;
  n = 0;
  A = NULL;
}

// This assumes that every element of the array is an
// element of the heap.
Heap::Heap(int* inArray, int inArraySize, int inHeapSize) {
  // TODO: initialize your class data members. An array dynamically allocated
  // as follows:
  // A = new int[size];
  // If you allocate an array like this you MUST deallocate it in your
  // destructor. This is done for you in the destructor below.

  arraySize = inArraySize;
  n = inHeapSize;
  A = new int[arraySize];
  A = inArray;
}

// Destructor. Cleans up memory.
Heap::~Heap() {
  delete [] A;
}

// Note: the function name is prefixed by Heap:: (the class
// name followed by two colons). Any function defined in
// the .cpp file must have this prefix.
int Heap::at(int i) const {
  return A[i];
}

int Heap::parent(int i) const{
    return (int) (i - 1) / 2;
}

int Heap::left(int i) const {
    return (i + 1)* 2 - 1;
}

int Heap::right(int i) const {
    return  (i + 1) * 2;
}

bool Heap::hasLeft(int i) const {
    int leftIndex = left(i);
    std::cout << "left index = " << leftIndex<< std::endl;
    return false; 
}

bool Heap::hasRight(int i) const{
    return false;
}

void Heap::maxHeapify(int i){

}
//
void Heap::buildMaxHeap(){

}



bool Heap::operator==(const Heap& rhs) {
  if (n != rhs.n) return false;
  for (int i = 0; i < n; ++i) {
    if (A[i] != rhs.A[i]) return false;
  }
  return true;
}

bool Heap::operator==(const int* rhs) {
  for (int i = 0; i < n; ++i) {
    if (A[i] != rhs[i]) return false;
  }
  return true;
}

std::ostream& operator<<(std::ostream& out, const Heap& h) {
  out << "[";
  for (int i = 0; i < h.n; ++i) {
    out << h.A[i];
    if (i < h.n-1) {
      out << ", ";
    }
  }
  out << "]";
  return out;
}

string toDotImpl(const Heap& h, int i) {
  using namespace std;
  stringstream ss;
  if (h.hasLeft(i)) {
    ss << toDotImpl(h, h.left(i));
    ss << "\"" << h.at(i) << "\" -> \""
       << h.at(h.left(i)) << "\"\n";
  }
  if (h.hasRight(i)) {
    ss << toDotImpl(h, h.right(i));
    ss << "\"" << h.at(i) << "\" -> \""
       << h.at(h.right(i)) << "\"\n";
  }
  return ss.str();
}

string toDot(const Heap& h) {
  using namespace std;
  stringstream ss;
  ss << "digraph G {\n";
  ss << "graph [ordering=\"out\"]\n";
  ss << "\"" << h.at(0) << "\"\n";
  ss << toDotImpl(h, 0);
  ss << "}\n";
  return ss.str();
}

and

Heap.h

#pragma once

// Provides I/O
#include <iostream>
// Provides size_t
#include <cstdlib>
// Provides INT_MAX and INT_MIN
// You can consider INT_MIN to be negative infinity
// and INT_MAX to be infinity
#include <climits>

//------------------------------------------------------------
// Heap class
//------------------------------------------------------------
class Heap {
 public:
  // Constructor
  Heap();

  // This constructor assumes that every element of the array is an
  // element of the heap.
  Heap(int* inArray, int inArraySize, int inHeapSize);

  // Destructor
  ~Heap();

  // Accesses an element of the array.
  int at(int i) const; 

  // Gets parent index of element at i
  int parent(int i) const;

  // Return element to the  left of i
  int left(int i) const;

  // Return element to the right of i
  int right(int i) const;

  // Checks if an element has a left child
  bool hasLeft(int i) const;

  // Checks if an elemnt has a right child
  bool hasRight(int i) const;

  // "Max heapifies" an array
  void maxHeapify(int i);

  // builds a max heap
  void buildMaxHeap();


  // Allows comparison between results
  bool operator==(const Heap& rhs);
  bool operator==(const int* rhs);

  // Useful for debugging. To use:
  //   Heap h;
  //   cout << h << endl;
  friend std::ostream& operator<<(std::ostream& out, const Heap& h);

 private:
  // The array
  int* A;

  // Size of the array
  int arraySize;

  // The number of elements in the heap
  int n;
};

// Useful for debugging. To use:
//   Heap h;
//   cout << h << endl;
std::string toDot(const Heap& h);

The code is called with I can include the entire main.cpp if needed but it has several hundred lines of just test cases that are commented out. int A[] = { 1, 2, 3, 4, 5, 6, 7, 8 }; Heap h(A, 8, 8);

if I comment out A = inArray; the program runs so I'm pretty confident that is where the issue is.

A is defined in Heap.h as `int* A;

Here is the full error:

*** Error in `./project': free(): invalid size: 0x00007ffd84786660 *** Aborted (core dumped)

this is probably quite a simple issue, but I can't figure out what is causing this since I believe this should allocate an array of size inArraySize of type int and then assign the given array inArray to A.

Full disclosure: this is part of an assignment so feel free to just point me in the right direction, but my professor is fine with us using stackoverflow as long as we site it.

Answer 1

You're trying to copy an array, but assigning pointers like that is not how to do it. There are various ways.

Standard C++:

#include <algorithm>

std::copy(inArray, inArray + inArraySize, A);

Using standard containers:

#include <vector>

std::vector<int> A(inArray, inArray + inArraySize);

Old style C way

memcpy(A, inArray, sizeof(int) * inArraySize);

Answer 2

Doing:

A = new int[arraySize];
A = inArray;

Is like doing:

i = 5;
i = 6;

The second assignment overrides the first one.

---

Hence as a result, the member variable A is pointing to the same memory block pointed by the input argument inArray.

If you haven't dynamically allocated this memory block (with new), then you cannot dynamically deallocate it (with delete).

Answer 3

The lines

A = new int[arraySize];
A = inArray;

are cause of two problems.

1. There is a memory leak. The value returned by new int[arraySize] is lost and cannot be deallocated.

2. If you are calling delete [] A in the destructor, that would be cause of the second problem.

   • If inArray was dynamically allocated and deallocated in the calling function, you will be calling delete on the same pointer twice.
   • If inArray was an array created in the stack, calling delete on it is also a problem. delete can be called only on memory that was returned by call to new.

Answer 4

A = inArray; is not doing what you think it is doing. This line does not copy inArray into the memory you allocated for A. Instead it changes A to point to a new location (the address of inArray), causing the previously allocated memory to leak. Later on when you call delete on A you'll be trying to free memory at inArray's address.

If you just want to copy an array, you could do something like

A = new int[inArraySize];
for (i = 0; i < inArraySize; ++i)
    A[i] = inArray[i];

Or better yet, with std::copy:

std::copy(inArray, inArray + inArraySize, A);