Passing by value in C++

Question

Here the object is passed by value, i don't understand the output of the program.

#include <iostream>
using namespace std;

class Sample
{
public:
    int *ptr;

    Sample(int i)
    {
        ptr = new int(i);
    }
    ~Sample()
    {
        delete ptr;
    }
    void PrintVal()
    {
        cout << "The value is " << *ptr;
    }
};

void SomeFunc(Sample x)
{
    cout << "Say i am in someFunc " << endl;
}

int main()
{
    Sample s1= 10;
    SomeFunc(s1);
    s1.PrintVal();
}

And the output is? Actually SomeFunc does not perform anything on x, so there is no difference if it will be passed by value or by reference... — Maxim Krizhanovsky, Aug 07 '11 at 13:36

score 8 · Answer 1 · edited May 23 '17 at 12:26

You have forgotten to write a copy-constructor. The compiler generates one for you which just copies the value of the pointer, not doing a deep copy. So, when the parameter you pass gets copied and destroyed, your original object tries to call delete the second time on the same pointer, which is undefined behavior.

Remember the Rule of Three.
In brief:
If you've a need to define either of copy-constructor, copy-assignment operator or destructor, then you probably want to define the other two as well. In this case your copy constructor should look something like this:

Sample(const Sample& other)
      :ptr(new int(*other.ptr));
{
}

Note: The op should also write the assignment operator. A simple copy and swap should do. — Martin York, Aug 07 '11 at 17:55

score 2 · Answer 2 · answered Aug 07 '11 at 13:36

2

The output here should be a crash I'd imagine.

The problem here is that when you pass by value your Sample object is copied. In your copied object you're not allocating a new ptr. So when the copy of s1 is destructed, it will delete the original s1.

Add a copy constructor below to get your desired behavior.

Sample(const Sample &s)
{
    ptr = new int(*(s.ptr));
}

answered Aug 07 '11 at 13:36

Joshua Weinberg

28,598
2
97
90

Note: The op should also write the assignment operator. A simple copy and swap should do. – Martin York Aug 07 '11 at 17:55

score 1 · Answer 3 · edited Aug 07 '11 at 14:18

Problem

The default copy constructor in your class provided by the compiler just copies the pointer instead of the integer that pointer points to. So now you have two pointers pointing to a single object. When one of the objects goes out of scope, the pointer inside the other object becomes dangling pointer. Trying to accessing a dangling pointer will almost always trouble you at run-time, like crashing your program.

int main()
{
    Sample s1= 10;
    // copying by value here creates a local Sample object inside SomeFunc
    // which points to the same heap-allocated integer. After it goes out
    // of its scope(leaves this function), that memory is released. Then 
    // "ptr" in s1 becomes a dangling pointer.
    SomeFunc(s1);
    s1.PrintVal();
}

Solution

If you have any class member pointer points to a heap-allocated buffer, you need to explictly overload copy constructor and assignment operator= to deep copy those pointer members, i.e., allocate new memory and copy the data those pointers point to.

score 0 · Answer 4 · answered Aug 07 '11 at 18:07

The problem is your class contains a pointer.

It is very rare for real code to need to contain OWNED pointers. They are usually wrapped in some utility class (like a smart pointer or a container) or more usually they are simple objects.

Your code should look like this:

class Sample
{
public:
    int val;    // No need to make it a pointer

    Sample(int i)
       : val(i)
    {}
    void PrintVal()
    {  cout << "The value is " << val;
    }
};

If you need a dynamically allocated object then it should be in a smart pointer (assuming you want a shared resource then it looks like this.

class Sample
{
public:
    std::shared_ptr<int>  ptr;

    Sample(int i)
        :ptr(new int(i))
    {}
    void PrintVal()
    {
        cout << "The value is " << *ptr;
    }
};

Assuming you are writing your own pointer wrapper (bad idea), but lets assume it is for practice with pointers. Then you need to obey the rule of three. This is because the compiler generated versions of copy constructor and assignment operator will not do the correct thing for OWNED RAW pointers. an OWNED RAW pointer is an a pointer that you own and are thus responcable for deleting.

There are two solutions to this problem. Make the copy constuctor and assignment operators private. This will stop your code working, so I assume that is not what you want. So we need to make them work. Since the shared case is covered above by std::shared_ptr<> here we will cover the non shared case of making copies.

class Sample
{
public:
    int *ptr;   // OWNED RAW POINTER

    Sample(int i)
       : ptr(new int(i))
    {}
    ~Sample()
    {
        delete ptr;
    }
    Simple(Simple const& copy)
        : ptr(new int (*(copy.ptr)))
    {}
    Simple& operator=(Simple rhs)    // Assignment uses the copy and swap idium.
    {                                // Notice the parameter is passed by value to get
        std::swap(ptr, rhs.ptr);     // an implicit copy using the copy constructor.
    }                                // Then we simply swap the pointers. The copy will be
                                     // destroyed with the old pointer.

    void PrintVal()
    {
        cout << "The value is " << *ptr;
    }
};

Passing by value in C++

4 Answers4