C++ How I can delete temp node mode from memory (to fix leak) and still maintain the node in "keys" list?

Question

Here is my code that has a list of structs. Each struct has id and vector. Essentially I have insert function that adds new node into keys list. Object is created inside the insert when the function is called. Once insertion is finished I want to avoid any leaks I hope for your help and suggestions!

struct Node {
    vector<int> data; //actual int vector
    int id;            //node id
};

list<Node *> keys;

//method{
Node *temp;
if (temp->id != key){
                temp = new Node;
                temp->id = key;
                temp->data.push_back(value);
                keys.push_back(temp);

I have troubles in that part of code:

                temp = nullptr; 
                delete(temp);   //that's how I'm trying to fix leak.
            }
//}

Valgrind:

==4199== HEAP SUMMARY:
==4199==     in use at exit: 620 bytes in 20 blocks
==4199==   total heap usage: 82 allocs, 62 frees, 75,452 bytes allocated
==4199== 
==4199== 4 bytes in 1 blocks are indirectly lost in loss record 1 of 4
==4199==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4199==    by 0x10B621: __gnu_cxx::new_allocator<int>::allocate(unsigned long, void const*) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10B0EA: std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10AA05: std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10A07D: void std::vector<int, std::allocator<int> >::_M_realloc_insert<int const&>(__gnu_cxx::__normal_iterator<int*, std::vector<int, std::allocator<int> > >, int const&) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x109A91: std::vector<int, std::allocator<int> >::push_back(int const&) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x1096F6: key_value_sequences::insert(int, int) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x108F93: main (in /home/admin/CLionProjects/A2/a2)
==4199== 
==4199== 36 (32 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 4
==4199==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4199==    by 0x1096C7: key_value_sequences::insert(int, int) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x108F93: main (in /home/admin/CLionProjects/A2/a2)
==4199== 
==4199== 296 bytes in 9 blocks are indirectly lost in loss record 3 of 4
==4199==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4199==    by 0x10B621: __gnu_cxx::new_allocator<int>::allocate(unsigned long, void const*) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10B0EA: std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10AA05: std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x10A07D: void std::vector<int, std::allocator<int> >::_M_realloc_insert<int const&>(__gnu_cxx::__normal_iterator<int*, std::vector<int, std::allocator<int> > >, int const&) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x109A91: std::vector<int, std::allocator<int> >::push_back(int const&) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x109627: key_value_sequences::insert(int, int) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x108F93: main (in /home/admin/CLionProjects/A2/a2)
==4199== 
==4199== 584 (288 direct, 296 indirect) bytes in 9 blocks are definitely lost in loss record 4 of 4
==4199==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4199==    by 0x10964F: key_value_sequences::insert(int, int) (in /home/admin/CLionProjects/A2/a2)
==4199==    by 0x108F93: main (in /home/admin/CLionProjects/A2/a2)
==4199== 
==4199== LEAK SUMMARY:
==4199==    definitely lost: 320 bytes in 10 blocks
==4199==    indirectly lost: 300 bytes in 10 blocks
==4199==      possibly lost: 0 bytes in 0 blocks
==4199==    still reachable: 0 bytes in 0 blocks
==4199==         suppressed: 0 bytes in 0 blocks
==4199== 
==4199== For counts of detected and suppressed errors, rerun with: -v
==4199== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

You need to give us enough code to replicate the leak. (But really, you should just use something other than a collection of raw pointers.) — David Schwartz, Aug 01 '18 at 04:29
Seems very likely that your memory leaks are caused by not having a correct destructor for `List`. You should read up on the [rule of three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). You cannot write a linked list class correctly without understandaing it. — john, Aug 01 '18 at 05:07
You have a list of pointers to objects you allocate with `new`, do you ever `delete` those objects? Even if the operating system free memory when the process exits, it's considered good practice to clean up after yourself before exiting the process. Such clean-up includes freeing memory. Every `new` needs a matching `delete`. — Some programmer dude, Aug 01 '18 at 05:48
@n.m. You're right I misread as `list` as `List`. Lack of a destructor to clean up memory is probably still the problem though. — john, Aug 01 '18 at 05:51

score 2 · Answer 1 · answered Aug 01 '18 at 03:46

2

Your delete temp does nothing, since you're effectively is doing delete nullptr.

But you should not change the assignment, because after the statement

keys.push_back(temp);

you have two pointers, both pointing to the very same object.

Then you do

delete temp;

which destructs that object, making the pointer in the list invalid. Any attempt to dereference the pointer in the list after that point will lead to undefined behavior.

In short, you should not delete the object.

If you have a memory leak, then maybe you're not removing and deleting the pointers from the list? Or perhaps it's a false positive and there is no actual leak. It's really impossible to say since you haven't shown any proof of a leak.

answered Aug 01 '18 at 03:46

Some programmer dude

400,186
35
402
621

Thanks! I understand that I'm basically deleting both temp and object under temp in keys. I'm still getting "leak_DefinitelyLost" regarding the new Node so I'll have to handle it somehow anyway. – opka Aug 01 '18 at 03:51
2

@opka Then please edit your question to show your leaks. Copy-paste (as text) the full and complete output from your leak-detector. Preferably also try to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve), and show the output from that MCVE. – Some programmer dude Aug 01 '18 at 03:53
I'm using CLion valgrind integration, but I did a check through terminal. Thank you for help! – opka Aug 01 '18 at 04:21
@opka When using raw pointers, you need to: (1) create an object with new (2) use that object (3) delete that object. In that order. Please show rhese three steps in your code. – n. m. could be an AI Aug 01 '18 at 05:46

score 0 · Answer 2 · answered Aug 01 '18 at 06:40

0

If you have a c++11 compatible compiler you should look at smartpointer. Otherwise @Some programmers dude is the correct answer.

answered Aug 01 '18 at 06:40

krjw

4,070
1
24
49

C++ How I can delete temp node mode from memory (to fix leak) and still maintain the node in "keys" list?

2 Answers2