memory allocation/deaalocation in calss

[asit]

I have a LinkedList class which has a function which allocates memory.

I need to add user defined copy constructor and assignment operator.

In case of assignment operator, I need to free the existing memory allocated(which is done by my destructor). Should I call destructor inside that assignment operator body ?

 

[Alf P. Steinbach]

I have a LinkedList class which has a function which allocates memory.

I need to add user defined copy constructor and assignment operator.

In case of assignment operator, I need to free the existing memory
allocated(which is done by my destructor). Should I call destructor
inside that assignment operator body ?

You should only explicitly call a destructor for an object constructed
in-place in existing storage (if that doesn't ring any bell, then it
means just don't explicitly call any destructor).

For an assignment operator, unless there are good reasons otherwise, use
the idiomatic implementation in terms of copy construction,

void operator=( MyList other ) { swapWith( other ); }

where swapWith is a MyList member function that swaps the contents of
the object that it's called on, with the contents of another such
object, guaranteed without failing.

It's simple and safe, and you then just need to implement custom copy
construction and the swapWith method.


Cheers & hth.,

- Alf

 

[Kevin McCarty]

I have a LinkedList class which has a function which allocates memory.

Why not just use std::list or (C++11) std::forward_list?

I need to add user defined copy constructor and assignment operator.

Yes. And also user-defined destructor, of course. If for some reason
you can't just use std:forward_)list.

In case of assignment operator, I need to free the existing memory allocated(which is done by my destructor). Should I call destructor inside that assignment operator body ?

No, absolutely not. There are very few situations in which a
destructor should be called explicitly, and those generally involve
manually cleaning up an object that was constructed in-place.

A good approach for the assignment operator is to implement a no-throw
swap member function that trivially exchanges the contents of two
objects, then implement assignment in terms of swap and the copy
constructor. For instance, presuming that you already have a sensibly
defined copy constructor and destructor:

void LinkedList::swap(LinkedList & other)
{
std::swap(_head, other._head);
std::swap(_tail, other._tail);
// or whatever
}

LinkedList & LinkedList:perator = (LinkedList other)
{
swap(other);
return *this;
}

Note that the argument of the assignment operator is passed by value
so that the copy constructor will be called automatically. Then the
temporary object "other" has its destructor called automatically at
the end of the assignment operator's body.

This idiom is automatically safe with respect to self-assignment. (It
is inefficient in that case ... but how frequently does self-
assignment happen in real-world code?) As well, if any exception is
thrown, it will be from the copy constructor, meaning that the
LinkedList to which the assignment was attempted does not get modified
before the throw, so is safe from harm.

- Kevin B. McCarty

 

[Ian Collins]

I have a LinkedList class which has a function which allocates memory.

I need to add user defined copy constructor and assignment operator.

In case of assignment operator, I need to free the existing memory allocated(which is done by my destructor). Should I call destructor inside that assignment operator body ?

A cleaner solution is to add a member function to manage the free and
call that from both places.

 

[asit]

Here is the code..

#include <iostream>

using namespace std;

class LinkedList
{
struct node
{
int data;
node *next;
};

node* header;

public:
LinkedList():header(NULL) {}
LinkedList(const LinkedList &cp)
{
node *t = cp.header, *t2;
header = NULL;
while(t != NULL)
{
t2 = new node;
t2->data = t->data;
t2->next = header;
header = t2;
t = t->next;
}

}
LinkedList& operator=(const LinkedList &cp)
{
if(&cp == this)
return *this;
//LinkedList();
node *t = cp.header, *t2;
header = NULL;
while(t != NULL)
{
t2 = new node;
t2->data = t->data;
t2->next = header;
header = t2;
t = t->next;
}

return *this;

}
~LinkedList()
{
node * t;
while(header != NULL)
{
t = header;
header = header->next;
delete t;
}
header = NULL;
}
void push(int data)
{
node *t = new node;
t->next = header;
t->data = data;
header = t;
}

void printList()
{
node *t = header;
while(t != NULL)
{
cout<<t->data<<"->";
t = t->next;
}
cout<<endl;
}
};

int main()
{
LinkedList list1;
list1.push(10);
list1.push(30);
list1.push(12);
list1.push(19);
list1.push(78);
cout<<"List1 : ";
list1.printList();

LinkedList list2;
list2 = list1;
list2 = list1;
cout<<"List2 : ";
list2.printList();


return 0;
}


Here is how DrMemory screams at me..

Dr. Memory version 1.4.6 build 2 built on Mar 7 2012 10:14:04
Application cmdline: ""C:\web\LoopLinkedList\bin\Debug\LoopLinkedList.exe""
Recorded 62 suppression(s) from default C:\Program Files\Dr. Memory/bin/suppress-default.txt

Error #1: UNINITIALIZED READ: reading 0x0022fd38-0x0022fd3c 4 byte(s)
# 0 ntdll.dll!TpDisassociateCallback +0x382 (0x7730f641 <ntdll.dll+0x2f641>)
# 1 ntdll.dll!TpCheckTerminateWorker +0x11 (0x7733b9ae <ntdll.dll+0x5b9ae>)
# 2 KERNELBASE.dll!TerminateThread +0x50 (0x7561dc35 <KERNELBASE.dll+0xdc35>)
# 3 profile_ctl
# 4 profil
# 5 moncontrol
# 6 _mcleanup
# 7 msvcrt.dll!isspace
# 8 msvcrt.dll!cexit
# 9 mainCRTStartup
#10 KERNEL32.dll!BaseThreadInitThunk
#11 ntdll.dll!RtlInitializeExceptionChain
Note: @0:00:01.789 in thread 4812
Note: instruction: cmp 0xfffffff8(%ebp) %ebx

Error #2: LEAK 8 direct bytes 0x003e14d8-0x003e14e0 + 32 indirect bytes
# 0 operator new()
# 1 operator new()
# 2 LinkedList:perator=() [C:/web/LoopLinkedList/main.cpp:40]
# 3 main [C:/web/LoopLinkedList/main.cpp:93]

DUPLICATE ERROR COUNTS:

SUPPRESSIONS USED:

ERRORS FOUND:
0 unique, 0 total unaddressable access(es)
1 unique, 1 total uninitialized access(es)
0 unique, 0 total invalid heap argument(s)
0 unique, 0 total warning(s)
1 unique, 1 total, 40 byte(s) of leak(s)
0 unique, 0 total, 0 byte(s) of possible leak(s)
ERRORS IGNORED:
68 still-reachable allocation(s)
(re-run with "-show_reachable" for details)
Details: C:\Users\asit\AppData\Roaming/Dr. Memory/DrMemory-LoopLinkedList.exe.1280.000/results.txt



Please help me to resolve this.

 

[asit]

I have a LinkedList class which has a function which allocates memory.

I need to add user defined copy constructor and assignment operator.

In case of assignment operator, I need to free the existing memory allocated(which is done by my destructor). Should I call destructor inside that assignment operator body ?

 

[Ian Collins]

Here is the code..

#include<iostream>

using namespace std;

class LinkedList
{
struct node
{
int data;
node *next;
};

node* header;

public:
LinkedList():header(NULL) {}
LinkedList(const LinkedList&cp)
{
node *t = cp.header, *t2;
header = NULL;
while(t != NULL)
{
t2 = new node;
t2->data = t->data;
t2->next = header;
header = t2;
t = t->next;
}

}
LinkedList& operator=(const LinkedList&cp)
{
if(&cp == this)
return *this;
//LinkedList();
node *t = cp.header, *t2;
header = NULL;

What happens here if the list isn't empty?

 

[Juha Nieminen]

LinkedList & LinkedList:perator = (LinkedList other)
{
swap(other);
return *this;
}

While this may be an easy way of implementing the assignment operator,
it has a performance issue: A copy of the other list will temporarily
exist at the same time as the current list, thus increasing memory
consumption. This can be an issue if the lists are very large.

The traditional way of implementing it is to *first* delete the current
list and *then* copy the other list to the current one. This way the
amount of memory required for the operation is minimized.

 

[Christopher Creutzig]

The traditional way of implementing it is to *first* delete the current
list and *then* copy the other list to the current one. This way the
amount of memory required for the operation is minimized.

The swap style has the benefit of being strongly exception safe.
Provided swap doesn't throw, which it never should do.


Christopher