-2

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;

RJ89
  • 3
  • 2
  • 2
    Never write such code even for studying purposes. [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) – 273K Feb 16 '23 at 07:27
  • 2
    [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) And [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) Also avoid plain non-owning non-smart pointers whenever you can, you almost never need it. Especially, a vector of pointers to a single `int`? That makes no sense even for practice or school assignments. – Some programmer dude Feb 16 '23 at 07:29
  • If you enter a pos greater than vector.size(), then you don't call delete and there is a memory leak. – kiner_shah Feb 16 '23 at 07:29
  • Oh and the error doesn't match the code you show. Please make sure you're showing us a proper [mre]. Then copy-paste the full and complete build-log into the question itself. And add comments on the lines in your code where you get the errors. – Some programmer dude Feb 16 '23 at 07:34
  • 1
    The best way to avoid leaks is to use the RAII idiom. In modern C++ this involves mostly delegating uses of new and delete to library facilities that are provided to manage dynamic memory rather than using them directly. I am not sure why std::vector is used here rather than std::vector. If you need the vector of pointers, it should have been std::vector< std::unique_ptr >. – Avi Berger Feb 16 '23 at 07:39
  • 1
    You need to `delete` all the vector elements, one by one. (On a side note, geeksforgeeks is well known for not being any good. Get [a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282).) – molbdnilo Feb 16 '23 at 08:13
  • 1
    when i see code on this site I also think "there is no way to fix it". I am not surprised anymore, the site is known to be full of bad code. Don't use it. – 463035818_is_not_an_ai Feb 16 '23 at 08:14

1 Answers1

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.

Swift - Friday Pie
  • 12,777
  • 2
  • 19
  • 42