destructor and assignment operator

I have the following destructor and assignment operator

Code:

---

//destructor
Vehicle::~Vehicle( )
{
 if (s != NULL){
          std::cout << "destructing " << s << std::endl;
        delete s;
 }
 else
        std::cout << "not destructing " << std::endl;       
}


// assignment operator
Vehicle& Vehicle::operator=( const Vehicle& other )
{
        if( &other != this )
        {
          s = other.s;
          L_ = other.L_;
          deltaS = other.deltaS;
        }
       
        return *this;
}

---

problem is it gives me an error of double free, how is this possible?? if s is already deleted previously s would be null and it wouldn't be deleted again... why is this?

You are just copying a pointer in your copy assignment operator. So, if the source object is destroyed, the object that its s points to is destroyed. It so happens that the destination object's s points to the same object, and its destructor tries to destroy that object again.

That's it.
You have to choose between copying the thing pointed at, or using a smart pointer to the item (or similar such behaviour), or using move semantics.
It's also usually a bad idea to test for self-assignment. See GotW for why
You also don't need to test for NULL prior to deletion.

Quote:

---

Originally Posted by laserlight

You are just copying a pointer in your copy assignment operator. So, if the source object is destroyed, the object that its s points to is destroyed. It so happens that the destination object's s points to the same object, and its destructor tries to destroy that object again.

---

so then I should allocate a new memory first before on my assignment operator and not just assigning it?

Quote:

---

Originally Posted by -EquinoX-

so then I should allocate a new memory first before on my assignment operator and not just assigning it?

---

The point is, if you have mutiple object responsible for deleting a single object then multiple delete's will invariably occur (you could instead use a pointer-to-a-pointer, which would allow you to delete, then nullify the shared pointer, but just keep in mind that you would need to also 'new' the pointed-to pointer as well). The best solution is a smart pointer, though (you might look into the boost for a good shared pointer implementation).

Quote:

---

...you also don't need to test for NULL prior to deletion.

---

But you should set it to 0 ot NULL after deletion.

Quote:

---

Originally Posted by Bubba

But you should set it to 0 ot NULL after deletion.

---

That is a good habbit, but then again it's also utterly pointless in a destructor. Still, it can't hurt...