how to use delete keyword correctly to avoid memory leaks in c++

Question

I see this code in geek for geeks and it has memory leak on 58 line as guided by valgrind , i don't see any way how to fix this to make it a better code, cause if we delete that new then assigned v[i] also delete and no value get inside it !! i am a beginner to coding!! here is the code

// C++ Program to create
// vector of pointer
#include<bits/stdc++.h>

using namespace std;

void insert_element(vector<int*>& v, int i)
{
    // declaration and input of values of elements
    int a;
    cin >> a;

    // allocating address to i element
    v[i] = new int(a);
}

void print_vector(vector<int*>& v)
{
    // printing elements of the vector
    for (int i = 0; i < v.size(); i++) {
        cout << *(v[i]) << " ";
    }

    cout << endl;
}

void delete_element(vector<int*>& v, int pos)
{
    // Out of limit positions
    if (pos <= 0 || pos > v.size())
        return;

    // converting position into index number
    pos = pos - 1;

    // free the space from pointer
    delete v[pos];

    // removing element from the vector
    v.erase(v.begin() + pos);
}

int main()
{
    cout << "Enter size of vector: ";

    // size of vector
    int n;
    cin >> n;

    // create a vector
    vector<int*> v(n, nullptr);

    cout << "Enter elements of vector: ";
    for (int i = 0; i < n; i++) {

        // inserting n elements inside v vector
        insert_element(v, i);
    }

    cout << "Before: ";
    // printing vector
    print_vector(v);

    cout << "Enter position to remove: ";

    int pos;
    cin >> pos;

    // delete element from pos position
    delete_element(v, pos);

    cout << "After: ";
    // printing vector
    print_vector(v);

    return 0;
}

i tried delete it but it says expected pointer in that place with delete keyword

error: type ‘class std::vector<int*>’ argument given to ‘delete’, expected pointer 15 | delete v;

Answer 1

The simplest way to avoid leaks is... not to use new\delete expressions anywhere outside of object construction and destruction. That's the part of RAII principle.

What valgrind reports, is that the function called on 58th line (insert_element) had created "leaked" free store memory, but that leak happens on exit from the main()function. The vector of pointers is destroyed, but the objects in free store aren't. In most cases it's a harmless scenario, but let be formal.

Our choices are

A.) Avoid extra allocation at all. It's not necessary here. we can use vector<int>. In this particular case it's a preferable solution. Usually the pointer's size is equal or greater in size than an int. Vector already works with free store.

B.) Use a vector that will delete elements automatically. Seriously, that's the responsibility of a container. But that would require multiple changes in code, because the type itself has changed. We can avoid that y declaring a type-alias.

    // Use it everywhere in code.
    using  MyVector = std::vector<std::unique_ptr<int>>

That's not a great solution. We overengineered the container also we can't actually make a copy of it. The latter is true for original code - the vector isn't really copyable, a second copy would have pointers referring to same memory, but clearing one copy would cause pointers in the other copy to "dangle" - to point at invalid memory where object no longer exists. You might need a smart pointer with reference counting instead -shared_ptr, even maybe a custom deleter and allocator. It's too much to store a single int.

C.) A "mechanist" solution: fix the problem by applying ducttape code where it is broken:

    // printing vector
    print_vector(v);

    for( auto &item : v)
        delete item;
    
    return 0;
}

PS. Be sure it's not a "ducktape code", i.e. a fragile solution where you patch every hole and apply it everywhere. Aside from the copyright issues with Ducktape brand, a badly written code, while badly maintained, quickly becomes an unreadable mess where following fixes may be utterly wrong and the project falls apart. Especially if one who does fixing didn't wrote the original.