Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #15355 > unrolled thread

memory allocation/deaalocation in calss

Back to article view | Back to comp.lang.c++

Contents

  memory allocation/deaalocation in calss asit <lipun4u@gmail.com> - 2012-04-13 14:02 -0700
    Re: memory allocation/deaalocation in calss "Alf P. Steinbach" <alf.p.steinbach+usenet@gmail.com> - 2012-04-13 23:35 +0200
    Re: memory allocation/deaalocation in calss Ian Collins <ian-news@hotmail.com> - 2012-04-14 09:36 +1200
    Re: memory allocation/deaalocation in calss Kevin McCarty <kmccarty@gmail.com> - 2012-04-13 14:35 -0700
      Re: memory allocation/deaalocation in calss Juha Nieminen <nospam@thanks.invalid> - 2012-04-14 05:27 +0000
        Re: memory allocation/deaalocation in calss Christopher Creutzig <christopher@creutzig.de> - 2012-04-26 19:46 +0200
    Re: memory allocation/deaalocation in calss asit <lipun4u@gmail.com> - 2012-04-13 15:17 -0700
      Re: memory allocation/deaalocation in calss Ian Collins <ian-news@hotmail.com> - 2012-04-14 11:29 +1200
    Re: memory allocation/deaalocation in calss asit <lipun4u@gmail.com> - 2012-04-13 15:31 -0700

#15355 — memory allocation/deaalocation in calss

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 ?

[toc] | [next] | [standalone]

#15356

On 13.04.2012 23:02, asit wrote:
> 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

[toc] | [prev] | [next] | [standalone]

#15357

On 04/14/12 09:02 AM, asit wrote:
> 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.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#15358

On Apr 13, 2:02 pm, asit <lipu...@gmail.com> wrote:
> 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::operator = (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

[toc] | [prev] | [next] | [standalone]

#15376

Kevin McCarty <kmccarty@gmail.com> wrote:
> LinkedList & LinkedList::operator = (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.

[toc] | [prev] | [next] | [standalone]

#15587

On 4/14/12 7:27 AM, Juha Nieminen wrote:

>   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

[toc] | [prev] | [next] | [standalone]

#15360

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::operator=()               [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.

[toc] | [prev] | [next] | [standalone]

#15367

On 04/14/12 10:17 AM, asit wrote:
> 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?


-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#15362

On Saturday, April 14, 2012 2:32:52 AM UTC+5:30, asit wrote:
> 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 ?

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c++

csiph-web