Reputation: 21
I have a small C++ program where I create two objects of a Person
class. This class has char *m_szFirstName
and char *m_szLastName
for data.
I then I assign one object to another, causing both object's data members to point to same location.
In the destructor, I delete what memory I had allocated for the first object and assign NULL
values to the pointers. Something like this.
if (m_szFirstName!= NULL)
{
delete [] m_szFirstName;
m_szFirstName = NULL;
}
Then when I go to delete memory for the second object, the check for NULL
does not work and when I delete memory, I get a crash. From debugger, it show that my pointer is not NULL
. It has 0xfeee
.
I am aware that memory was already deleted before and should not be delete. However, I don't know how to check whether I should delete memory or not.
Upvotes: 2
Views: 958
Reputation: 206508
Reason for Crash:
You should follow the Rule of Three to avoid this problem of dangling pointers.
If you need to explicitly declare either the destructor, copy constructor or copy assignment operator yourself, you probably need to explicitly declare all three of them.
In your case You do not define a copy assignment operator thus leading to shallow copy of the pointer.
Suggested Solution:
If you can use std::string
instead of char *
just simply use std::string
, it has the first and foremost preference over any kind of silly pointer stuff.
You can avoid all the funky pointer stuff by using std::string
.
If you can't read on and the following suggestion applies to any class pointer member in general.
Note that the ideal solution here is to not use raw pointers at all, Whenever you use raw pointers you are forced to manually manage the resources acquired by them, it is always difficult and error prone to manually manage resources.So the onus is to avoid it.
To do so, You should use a Smart pointer which will manage the dynamic memory of the pointer implicitly.Using a smart pointer will ensure that the dynamic memory is implicitly released after usage & you do not have to manually manage it.
The scenario you have is the very reason that in C++ you should rely on RAII rather than manual resource management & using a Smart pointer is the way to go about in your case.
Caveat:
Note that I restrained myself from suggesting which smart pointer to use because the choice rather depends on the ownership and lifetime of the elements involved, which is not clear from the data provided in the Question.So I will suggest reading,
Which kind of pointer do I use when?
to make your choice of the smart pointer to use.
Upvotes: 4
Reputation: 182753
Just stop setting pointers to NULL when you have deleted the object. As you can see, it just leads to pain. You cannot assume that because the pointer is not-NULL, it has not been deleted already.
You can use any sensible pattern you want to avoid this problem. For example, Boost's shared_ptr
is a great choice.
Upvotes: 0
Reputation: 11933
Your problem is a classic C/C++ problem which is known as "Dangling Pointer" problem. Dereferencing the dangling pointer has resulted in the crash. The problem is about reference counting. Once you assign the same memory to the second pointer then the reference count should be 2. So if you delete one pointer reference count should become 1 and your memory should not be deallocated or freed unless count is 0. At 0 it can be garbage collected.
Now there are good answers above to solve your problem. Since you are using C++ so you can use something like an auto_ptr (OR shared_ptr). They provide what I had mentioned above, the reference counting stuff and even you need not worry about deleting or freeing your objects, these classes would take care. They work on simething known as RAII pattern where a destructor is auto-magically called when the object on the stack goes out of scope.
Upvotes: 0
Reputation: 541
When you have two pointers pointing to the same location (after you assigned the first one to the second one), you have two pointers pointing at the same address. Deleting one frees the memory pointed to by both of them. But setting one to NULL
doesn't alter the other pointer. The same happens if you have two integers, for example.
int a = 3;
int b = a;
Now if you run
a = 0;
the value of b
doesn't change. The same way your second pointer doesn't change when you alter the first one (but when you alter the memory pointed to by either pointer, you can see the effect through the other as well).
Upvotes: 0
Reputation: 741
With
if (m_szFirstName!= NULL)
{
delete [] m_szFirstName;
m_szFirstName = NULL;
}
You only set m_szFirstName to point to NULL, not m_szLastName. This means you have to have some way to keep track of the fact they point to the same location. Is there a reason they point to the same location? Could you copy the name instead of pointing the pointers to the same place?
If you need the two pointers to shared the same data, I would have a look at std::tr1::shared_ptr, which will solve this issue for you by keeping track of the number of references and deleting when the number of references reaches 0.
Upvotes: 1
Reputation: 9466
Don't delete it again, if (m_szFirstName == m_szLastName). But this will give you a memory leak (when you assign one pointer to other).
Upvotes: 0