CODEWITHSUNDEEP
c++ooppointers
Vivek MVK
Vivek MVK

Reputation: 1179

Deleting a pointer in destructor C++

I am using a class from a library. Let it be A, and it has a character pointer "token"

My code:

void someFunction()
{
    A a;
    cout<<a.token;
    anotherFunction(a);
    cout<<a.token;  //  line 4: now token became invalid [1]
}

void anotherFunction(A copyOfA);
{
   //  doing something
}  //  on exit destructor of copyofA will be called

[1] Why did it become invalid: class A is as follows:

class A
{
    char *token;
    public:
    A()
    {
        token = GetRandomToken();   // GetRandomToken will return a 'new Char' array
    }
    ~A()
    {
        if(token != NULL)
        {
            delete[] token;    // it is A's responsibility to delete the memory it created
            token = NULL;
        }
    }
};

in anotherFunction when the destructor of copyOfA is called token got deleted. So at line 4, token is invalid because both a.token and copyOfA.token both pointing to same address.

What is the solution, in following case:

case 1: class A is in a given library: So I can't modify it.

case 2: if I can modify class A: What will be the good way to handle this?

I know, if anotherFunction is called by passing reference, I won't have hit this problem. But what if I have to keep a copy of the object at some point?

Check sample code here: https://ideone.com/yZa4k4

Upvotes: 4

Views: 2333

Answers (2)

Reinhold
Reinhold

Reputation: 584

If your example is accurate then class A doesn't have a proper copy constructor and therefore deletes the token for both instances. Which leads to a double delete because the pointer in the first instance didn't change.

Upvotes: -1

pptaszni
pptaszni

Reputation: 8218

If you cannot modify class A, then you should avoid copying it. I think that the most safe way to do it is to allocate object of class A dynamically:

void anotherFunction(std::shared_ptr<A> aPtr)
{
    // please also note that in your case token is PRIVATE
    std::cout << aPtr->token << std::endl;
}

std::shared_ptr<A> aPtr(new A);
std::cout << aPtr->token << std::endl;
anotherFunction(aPtr);

Or if you insist on stack allocation, you should change anotherFunction signature to:

void anotherFunction(const A& a)
{
    std::cout << a.token << std::endl;
}

to pass your argument by const reference (avoid copy-constructor).

Now, if you CAN modify your class A, you should apply the rule of three/five/zero, because you have non-trivial destructor. The lazy way to do this would be to just declare other constructors as deleted (then, like in your example, you cannot copy your A object, but also you have a guarantee that no one will attempt to do so):

class A
{
    public:
    // for this example purpose I made token PUBLIC, but it is a bad idea in general
    char *token;
    A()
    {
        token = GetRandomToken();   // GetRandomToken will return a 'new Char' array
    }
    ~A()
    {
        if(token != NULL)
        {
            delete[] token;    // it is A's responsibility to delete the memory it created
            token = NULL;
        }
    }
    A(const A& other) = delete;
    A(A&& other) = delete;
};

Or, if you are not lazy, you can actually think about how to copy the memory from token pointer in one object to another object - it is up to you how you would implement it. That depends on the requirements and implementation of GetRandomToken.

Upvotes: 3

Related Questions