-1

I try to clean the memory to delete pointers. But the last instruction (delete array[i]) generate this error :

pointeurs-tableau(14994,0x110a67600) malloc: *** error for object 0x7ff7bb760760: pointer being freed was not allocated

But I don't know why.

#include <iostream>
#include <random>
#include <ctime>
using namespace std;

int main()
{
    default_random_engine gen(time(0));
    uniform_int_distribution<> dis(1, 100);

    const int max = 20;
    
    int* array[max];
    // un tableau normal
    int val[max];

    for (int i = 0; i < max; i++) {
        val[i] = dis(gen);
    }

    for (int i = 0; i < max; i++)
    {
        array[i] = &val[i];
    }

   //... modifying function on the array

    // show the array

    for (int i = 0; i < max; i++) {
        cout << val[i] << " ";
    }
    

    // deleting pointeurs (problematic !!!!)
   for (int i = 0; i < max; i++)
   { delete array[i];}
   delete[] array;
  return 0;
}

Could you help me, please?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • 10
    I see no calls to `new` anywhere in the code you posted. Thus issuing `delete` is invalid. – PaulMcKenzie Dec 23 '22 at 16:23
  • 1
    For that matter, `delete[] array;` is invalid because `array` was not created by `new[]`. You have variables with automatic storage duration that clean up after themselves. – Nathan Pierson Dec 23 '22 at 16:25
  • If you never use `new` then you don't have to use `delete`. Same is true for `new[]` and `delete[]`. The error message tells you exactly what is wrong *pointer being freed was not allocated*. – john Dec 23 '22 at 16:26
  • 1
    You don't need to and can't `delete` anything that wasn't created with `new`. Just get rid of that last `for` loop and the `delete[]` after it. It doesn't belong there. You don't need to delete anything. – user17732522 Dec 23 '22 at 16:27
  • *I try to clean the memory to delete pointers* -- If you took a logical look at your code, what memory would you be "cleaning"? The pointers point inside of a regular `int` array, not at memory that was dynamically allocated. You wouldn't issue a `delete` on a regular array, would you? So that is what your attempt was doing all along, and you see how it could never make sense. – PaulMcKenzie Dec 23 '22 at 16:45
  • Learn about address sanitizer. – Jesper Juhl Dec 23 '22 at 16:52
  • [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – Jesper Juhl Dec 23 '22 at 16:54
  • Ouch, this code is just *wrong*. Stop with the manual memory management already. – Jesper Juhl Dec 23 '22 at 16:55

1 Answers1

1

delete operator must be used only on a dynamically allocated memory (with the new).

In your case you have an array int val[max]; with automatic storage duration. And array of pointers int* array[max]; also with automatic storage duration.

Automatic storage duration means memory for arrays int val[max]; and int* array[max]; will be allocated when execution enter in the scope they declared and will be freed when execution lives the scope (at your case main function).

But when you trying to call delete array[i]; you force compiler to attempt clear element from int val[max] onto which array[i] pointing to. But it can't do that because this value never have been allocated on the heap with new.

// Edit

As you mentioned in comment to this answer you added changes to your code:

int **array = new int*[max];
for (int i=0; i < max; i++) {
    array[i] = new int;
}

An still have the same error;

The reason behind it most likely is that you still have this cycle

for (int i = 0; i < max; i++) {
    array[i] = &val[i];
}

int** array; item: array[i] - is a pointer. You allocated memory on the heap and stored address of this memory into that pointer:

array[i] = new int;

Lets say new int; returned address ADDRESS_FROM_HEAP Then you took address of int val[max]; by &val[i] and assigned it to the pointer array[i] = &val[i]; Lets say &val[i] equal to ADDRESS_FROM_STACK Then you trying to delete array[i]; But at this moment array[i] == ADDRESS_FROM_STACK; Not only delete unable to free memory on ADDRESS_FROM_STACK, you also lost ADDRESS_FROM_HEAP and memory by this address will not be freed.

If you would change your loop to this

for (int i = 0; i < max; i++) {
    *array[i] = val[i];
}

You will store a COPY of val[i] in the memory pointed by array[i] pointer.

But by looking at your code I can't honestly see why you allocate any memory dynamicly.

If you will just leave code as it is in your original question and just remove all delete statements, it will work just fine.

Yuri Dolotkazin
  • 480
  • 4
  • 13
  • Thank Yuri you for your answers. then i did some modification to create my array of pointer like this int **array = new int*[max]; for(int i=0;i – Christophe Martin Dec 23 '22 at 17:29
  • @ChristopheMartin look it is not about pointers it self. It is abou how you allocate and free the memory. When you write int* something; in the scope of the function (inside the body of the function)memory for that pointer allocated on the stack. When execution come to your function, stack pointer will be moved a to make a room for this pointer. If you then assign addres of the local variable(lets say int variable; which is declared right after this pointer in the function scope)to this pointer (something = &variable)then you don't have to use delete to free the memory you pointer points to. – Yuri Dolotkazin Dec 24 '22 at 13:38
  • Pointers don't steal ownership from the variables they point to. On other hand, if you will allocate memory on the heap (something = new int;) then new will return a new address for the memory allocated on the heap. And this memory will not been returned back to the heap when int* something will go out of scope (when function execution ends). Only for that particular reason you should use delete (delete something;) to return memory back to the heap. – Yuri Dolotkazin Dec 24 '22 at 13:43
  • In your case you taking address of the array element which leaves on the stack (array[i] = &val[i]) and then trying to return this address from stack to the heap when you call delete array[i]; Lets see. array[i] has a pointer type. It is the variable which stores an address to another int variable. In your case by doing &val[i] you are taking address of the i-th int value from array int val[max]; which is array leaving on the stack. Then you do delete array[i]; delete looks at the address stored in array[i] and trying return memory on this address to the heap. But this memory belongs to stack. – Yuri Dolotkazin Dec 24 '22 at 13:53
  • If you only purpose to make delete work, you have to allocate memory on the heap: int* array = new int[max]; for the whole array and free it with delete[] array; as whole because you allocated it as whole. Or you may allocate memory for each element separately: int* array[max]; and for(int i = 0; i < max; ++i) array[i] = new int; then each pointer in array of pointers called array will store pointer it's own heap allocated int. Then you would need to call delete in the same way in the cycle for each element. for(int i = 0; i < max; ++i) delete array[i]; but not for array it self. – Yuri Dolotkazin Dec 24 '22 at 14:03
  • Rules is simple: 1) delete MUST be called exactly the same amount of time as new was called. 2) delete[] MUST be called if pointer store memory allocated with new []. 3) delete MUST be called if pointer store memory allocated with new without []. – Yuri Dolotkazin Dec 24 '22 at 14:04
  • If you wrote this: int **array = new int*[max]; for(int i=0;i – Yuri Dolotkazin Dec 24 '22 at 14:20
  • So instead of taking address of the val[i] by &val[i], you better take a value it self val[i] and store a copy of it in your heap allocated array[i]. array[i] = val[i]; I will update the answer when I will be at home. – Yuri Dolotkazin Dec 24 '22 at 14:24
  • 1
    Thank you very much Youri for this very detailed answer. It helps me a lot to better understand the manipulation of these entities. I will continue my tests and my learning in a more enlightened way now. I know some find my code useless, poorly constructed that I don't need the pointers in this case. But this code is just an example to help me better understand primitive pointers. And I don't like to evade a subject that I don't understand. Thank you again for your friendliness. – Christophe Martin Dec 25 '22 at 05:51