C++ dynamic array, increasing capacity

Question

I'm trying to implement a dynamic array and here is my function for increasing the capacity

int* changeCapacity(int *arr, int length, int newCapacity) {
    int *newArr = new int[newCapacity];

    if(length > newCapacity){
        return 0;
    } else {
        for(int i = 0; i < length; i++){
            newArr[i] = arr[i];
        }
        delete[] arr;
        arr = newArr;
        return arr;
    }
}

This is the error i get:

speicher(2465,0x7fff7cfc2310) malloc: * error for object 0x7f9742403910: pointer being freed was not allocated * set a breakpoint in malloc_error_break to debug

i'm calling it like this:

int* addElement(int *arr, int& length, int& capacity, int val){
if(length >= capacity){
    capacity = capacity * 2;
    changeCapacity(arr, length, capacity);

    arr[length] = val;
    length += 1;
    return arr;
}else{

    arr[length] = val;
    length += 1;
    return arr;
}

}

Answer 1

Right now you are changing the address of arr, in which you have to pass the pointer by reference. Do this:

int* changeCapacity(int *&arr, int length, int newCapacity)

Answer 2

This is somewhat a guess based on the error message, but you have shown:

int* addElement(int *arr, int& length, int& capacity, int val)
{ //...
  changeCapacity(arr, length, capacity);
  //...
}

which calls:

int* changeCapacity(int *arr, int length, int newCapacity)
{ //...
  delete[] arr;
  //...
}

However, the original source of the arr argument to addElement() is unknown given the code you have posted so far. Are you by chance doing something like this:

foo()
{ int array[N];
  //...
  addElement(array, ...);
  //...
}

or maybe calling addElement() with a global array variable? In either of those cases, the original array is not allocated via a new[] to match the delete[], and this would appear to be what the runtime library is complaining about. The value of the pointer stated in the error message tends to make me think it was allocated on the stack initially.

Of course the other issues, like not capturing the return value of changeCapacity() and/or addElement(), and the possibility that changeCapacity() might return a NULL pointer are still valid as well, and should be fixed.

Answer 3

Here is a better way of doing it. Everything is well explained in the comments for anyone who wants to learn:

        #include <iostream>
        using namespace std;

        int* changeCapacity(int *arr, int length, int newCapacity);
        int* addElement(int *arr, int& length, int& capacity, int val);

        int main(){
            int length = 0; // no inital elements in array
            int capacity = 1; // initial capacity is one
            int* arr = new int[capacity]; // allocating space for values
            int* temp; // pointer for storing temporary values
            /* loop for adding elements to the array */
            for(int i=0;i<21;i++){
                temp = addElement(arr,length,capacity,i); // adding an element to the array
                if(temp == NULL) { // checks if execution was successful
                    cout<< "NULL returned...\n Exiting Now...";
                    return 0; // exits the program on failure
                }
                arr = temp; // changing the value of arr
            }
            /* loop for printing the array */
            for(int i=0;i<length;i++){
                cout<<arr[i]<<" ";          
            }
            return 0;
        }
        /* function for increasing the capacity of array */
        int* changeCapacity(int *arr, int length, int newCapacity) {
            int *newArr = new int[newCapacity]; // definging a new array

            if(length > newCapacity){ // checking if the length of the array is valid
                cout<< "invalid length of array\n";
                return NULL;
            } else {
                /* loop for transferring values to the new array */
                for(int i = 0; i < length; i++){
                    newArr[i] = arr[i];
                }
                delete[] arr; // deleting the old array (clears the memory of the old array)
                // arr = newArr; removed as this is not needed
                return newArr; // returns the new array
            }
        }

        /* function for adding a new element to the array */
        int* addElement(int *arr, int& length, int& capacity, int val){
        if(length >= capacity){ // checks if the array has space for storing the given value or not
            capacity = capacity * 2; // doubles the capacity of the array
            int* temp = changeCapacity(arr, length, capacity); // changes the size of the array to the new one
            if(temp == NULL){ // checking if a null was returned
                cout<< "Failed to change capacity\n";
                return NULL; // returning  NULL
            }
            arr = temp; // the value of arr was not changed in your code (problem corrected)
            arr[length] = val; // stores the value in the array
            length += 1; // increasing the number of element count of the array
            return arr; // returns the new array
        }else{
            arr[length] = val; // stores the value in the array
            length += 1; // increasing the number of element count of the array
            return arr; // returns the new array
        }
        }

Answer 4

I assume that your problem must come from two things IMO :

First:

changeCapacity(arr, length, capacity);
arr[length] = val;

Here you doesn't get the new arr value (returned by changeCapacity()). So your function addElement() will return Wrong pointer and on your next addElement() that will do free memory corruption.

Why do you have to get the new arr value?

You do the same as here

a = 1;
changeVar(a);
// value of a here?

int changeVar(int a)
{
   a = 5;
   return (a);
}

How will be the value of a? 1 because the argument of changeVar is a local variable.

Second:

You give a NULL value on your addElement() function.