5

here is my c++ code :

  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();
    }

it returns me the output like :

Say i am in someFunc 
Null pointer assignment(Run-time error)

here As the object is passed by value to SomeFunc the destructor of the object is called when the control returns from the function

should i right ? if yes then why it is happening ? and whats the solution for this ???

kennytm
  • 510,854
  • 105
  • 1,084
  • 1,005
Milan Mendpara
  • 3,091
  • 4
  • 40
  • 60
  • 1
    See: [The rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Benjamin Lindley Jan 23 '12 at 17:11
  • 1
    the compiler is building a default copy-constructor for you, which you need to override whenever you allocate memory in your class (along with the assignment operator) - look at the Rule of Three link below – kfmfe04 Jan 23 '12 at 17:12

5 Answers5

5

You usually need to obey the Rule of Three since you have an pointer member.
In your code example to avoid the Undefined Behavior you are seeing:

Replace the need to in first statement by must.

Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • No need to obey if you know what you are doing. A slightly less forcible wording might be nice. – pmr Jan 23 '12 at 17:11
  • @pmr: I hope that is less forcing enough. – Alok Save Jan 23 '12 at 17:13
  • @pmr: This might be enough to constitute a new question, but in which cases would you *not* obey the Rule of Three because you know what you are doing? Or are you just talking about cases where you make the other members private and therefore inaccessible? – Cody Gray - on strike Jan 23 '12 at 17:33
  • @CodyGray Quick add hoc functors that should be copyable and never outlive the object (equal to capturing a pointer with a lambda). Or when I'm forced to use `Qt`. – pmr Jan 23 '12 at 17:34
  • @pmr: Unless those functors are doing something weird, then they would just have the implicitly-generated destructor and copiers; and so they would still obey the rule of three, which says that if *any* need to be user-defined, then usually *all three* need to be. – Mike Seymour Jan 23 '12 at 18:06
  • @MikeSeymour I believed this answer and thought the pointer member triggers the rule. My fault entirely. – pmr Jan 23 '12 at 19:33
  • @pmr:The rule is clear enough,*If you define any one of copy constructor, copy assignment operator or destructor yourself then you most likely need to define the other two too.* Notice the *most likely*, In this case, the dynamic allocation & deletion of the pointer member necessitates following the Rule of Three.Hope that clears it for you if any doubts due to my answer. – Alok Save Jan 24 '12 at 02:40
5

Sample is passed by value to SomeFunc, which means a copy is made. The copy has the same ptr, so when that copy is destroyed when SomeFunc returns, ptr is deleted for both objects. Then when you call PrintVal() in main you dereference this invalid pointer. This is undefined behavior. Even if that works then when s1 is destroyed ptr is deleted again, which is also UB.

Also, if the compiler fails to elide the copy in Sample s1= 10; then s1 won't even be valid to begin with, because when the temporary is destroyed the pointer will be deleted. Most compilers do avoid this copy though.

You need to either implement copying correctly or disallow copying. The default copy-ctor is not correct for this type. I would recommend either making this type a value type (which holds its members directly rather than by pointer) so that the default copy-ctor works, or use a smart pointer to hold the reference so that it can manage the by-reference resources for you and the default copy-ctor will still work.

One of the things I really like about C++ is that it's really friendly toward using value types everywhere, and if you need a reference type you can just wrap any value type up in a smart pointer. I think this is much nicer than other languages that have primitive types with value semantics but then user defined types have reference semantics by default.

bames53
  • 86,085
  • 15
  • 179
  • 244
  • It might be worth adding that `Sample s1 = 10;` can also call the copy constructor. – James Kanze Jan 23 '12 at 17:17
  • @Milracle: you can read more about this ["rule of three"](http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). – André Caron Jan 23 '12 at 22:07
  • @AndréCaron: I think our very own SO c++-Faq is a better read - [Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Alok Save Jan 24 '12 at 02:46
1

Since SomeFunc() takes its argument by value, the Sample object that you pass to it is copied. When SomeFunc() returns, the temporary copy is destroyed.

Since Sample has no copy constructor defined, its compiler-generated copy constructor simply copies the pointer value, so both Sample instances point to the same int. When one Sample (the temporary copy) is destroyed, that int is deleted, and then when the second Sample (the original) is destroyed, it tries to delete the same int again. That's why your program crashes.

You can change SomeFunc() to take a reference instead, avoiding the temporary copy:

void someFunc(Sample const &x)

and/or you can define a copy constructor for Sample which allocates a new int rather than just copying the pointer to the existing one.

Wyzard
  • 33,849
  • 3
  • 67
  • 87
0

When you pass the argument for the function it's called the copy constructor, but you don't have one so the pointer is not initialised. When it exits the function, the object is calls the destructor to delete the unitialised pointer, so it thows an error.

Andrei
  • 100
  • 2
  • 11
0

Instead of

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

try to use

int main()
{
Sample* s1= new Sample(10);
SomeFunc(*s1);
s1->PrintVal();
}
kochobay
  • 392
  • 1
  • 7