deleting dangling pointers

[maharjun]

if a pointer is a dangling pointer and one applies delete[] to it. is the result defined or not. i basically have the following situation

class A
{
    private:
        int *p;
        //other data members
    public:
        //rest of class
}

i intend to initialise p in the constructors. but we know that constructors are also called when the = operator is encountered(or atleast thats what i observed). so when we get the arguments from an = assignment, it is possible that p already points to some valid location and hence, while reassigning the class, would have to be deleted to avoid memory leak.

but when the constructor is first called i.e. when object is created, at that time the pointer would not point to anything valid neither would it be NULL. hence there is a very high chance that the delete will execute even if i bracket it with an if check to check for null pointer.

hence it would mean that i am applying the delete operator on a wild/dangling pointer. will this lead to any instability etc.?

i know that this can be averted if i define the = operators. but im looking for ather options(if exist).

thanks for help.

[Fbody]

I think you'll have to share a little more of your code so that we can see exactly what's going on.

What do you initialize 'p' to? Are you initializing it to NULL or an actual object/value?

The assignment operator should only call a constructor if you use a pass by value in the overloaded definition (which you really shouldn't) or if it is part of an implicit initialization statement (i.e. int i = 2; is an implicit initialization). When you pass by value (again, which you shouldn't), it creates a temporary object. This temporary object would be created with a copy constructor. Since p is a pointer, you'll want to create a proper copy constructor, overloaded assignment operator, and destructor.

Based on the situation you are describing, it sounds like you didn't write your copy constructor and/or assignment operator properly, if at all.

[maharjun]

I'm sorry if i haven't been clear. this isn't my actual class. my actual class is too big. this was just a sample that i gave.

by the way in my class i have defined and successfully coded a copy constructor(among other constructors) and a destructor as well. however, i HAVE NOT DEFINED an = operator i was 'looking for other options' i.e a way to get around the problem by using the fact that constructors are called on assignment.

im trying to do both things with p. i.e. when the default constructor is called, make it NULL. and when another constructor is called, assign it accordingly

ill give a better sample

class A
{
    int *p;
    //other members
    public:
    A(&A)
    ~A();
    A();
    A(int a);
    //other functions
}

A::A()
{
    p = NULL;
    //other initializations
}
A::A(int a)
{
    if(p!=NULL) delete p;   //this is the line that's worrying me
                            //it is necessary to avoid memory leak in case this
                            //constructor is called after the object has been defined
                            //e.g. by using assignment operator
    p = new int;
    *p = a;
}

so we have the following cases.

CASE I

A a1;                  //calls default constructor and sets p = NULL
    a1 = 3;                //A::A(int) called. no problems. because since p is null
                           //hence delete is not executed

CASE II

A a1(3);               //this worry's me since the delete operation is being 
                           //performed on the dangling pointer that is created just
                           //as the object is created.

by the way, my question wasn't answered. DOES PERFORMING DELETE OPERATION ON A DANGLING POINTER LEAD TO INSTABILITIES? i know that delete creates dangling pointers. but what if a delete is performed on them? is it dangerous? if it doesn't lead to instabilities then it is a point to be noted that the if condition to check if it's a null pointer wouldn't be required.)

the reason this seems to be possible that it would not lead to instability is that the delete operator has access to heap memory and thus it shouldn't touch memory that is not allocated. but however i would tremendously appreciate any clarification on the issue as what i said above was merely speculation. thanks

[Fbody]

Yes, performing delete on a dangling pointer can lead to issues:

...
The delete operator must be used only with a valid pointer previously allocated by using new. Using any other type of pointer with delete is undefined and will almost certainly cause serious problems, such as a system crash.
...

But, if your class is written correctly, you shouldn't have to worry about it in the first place.

Honestly though, why would you delete the pointed object then create a new one? What is the point of that? Why make it hard on yourself? Why not just modify the existing one? It's not a memory leak if you modify an existing object.

A::A(int a)
{
  if(p!=NULL)
    *p = a;
  else
    p = new int(a);
}

I'm not too sure you understand how this code should work.

A a1;                  //calls default constructor and sets p = NULL
    a1 = 3;                //A::A(int) called. no problems. because since p is null
                           //hence delete is not executed

You are corrrect that this does call the default constructor, which makes A::p NULL. However, this does not call a constructor when you perform the assignment, or at least it shouldn't. It should call an assignment operator. Have you written an overloaded assignment operator?

[mike_2000_17]

Ok.. here's the deal.
When you hold a pointer within an object you need to clarify "who" owns the pointed-to memory. If the object owns the pointed-to memory, then you have to perform a deep-copy of the object if you assign it to another one and in this case there is no other way but to define an assignment operator for that class. Look up the topic of shallow-copies vs. deep-copies.

Yes, deleting a dangling pointer is a VERY BAD IDEA. And by the way, in the simple example you posted, you do delete a dangling pointer:

class A
{
    int *p;
    //other members
    public:
    A(&A)
    ~A();
    A();
    A(int a);
    //other functions
}

A::A()
{
    p = NULL;
    //other initializations
}
A::A(int a)
{
    if(p!=NULL) delete p;   //at this line, the value of p is not initialized at all
                            // because a constructor is always called on a "fresh"
                            // object and only called once. So this line will delete
                            // an uninitialized pointer (BOOM.. CRASH!!!)
                            // note: some compiler will issue a warning here.
    p = new int;
    *p = a;
}

Now, I need to explain the sequence of event when you assign a value (3) to the object of class A. Since the object "a1" has been constructed already, its constructor is not called again for sure. But a constructor is called to create a temporary object of class A that is constructed with A::A(3). This is because you cannot assign an int to an object of class A, but you can create one the available "implicit" constructor and then copy the temporary object to the object "a1". The problem here, and it is a big problem, is that since you have no assignment operator implemented to do this copy from temporary to "a1", the compiler provides one for you and it will be "shallow". A shallow-copy is essentially like a literal memory copy (not exactly), for this simple example the compiler-provided assignment operator is equivalent to this:

A& A::operator =(const A& rhs) { //DEFAULT: SHALLOW
  p = rhs.p; //only copies the value of the pointer, does not copy memory pointed-to by rhs.p.
  return *this;
};

1) If the class A claims _ownership_ of the memory pointed-to by its member p, then you should delete it in the destructor and thus there cannot be another object that has a copy of that pointer.
2) If the class A does not claim _ownership_ of the memory, then you shouldn't delete it in the destructor and neither should you allocate it within the class but rather provide it externally (through a constructor or otherwise) such that it can be deleted externally.
3) A third case, which is a real pickle is if class A claims a shared _ownership_ over the memory. Then, I'm afraid it is no longer possible (at least within a reasonable amount of effort) to implement this with simple pointers.

Fortunately, there is a nice feature called smart pointers which are originally from Boost, now part of the std::TR1 namespace, and probably soon in the std namespace with the coming C++0x standard. The above three cases can all be implemented with three smart pointers: unique_ptr, shared_ptr, weak_ptr. unique_ptr will give exclusive ownership of the pointed-to memory, i.e., it cannot be copied but only moved (meaning that assigning one unique_ptr to another will set the lhs to the address and set the rhs to null), this "enforces the law" of case 1. weak_ptr can be used to grant access to the memory to the object of class A, but no ownership of it (only a temporary ownership via the creation of a temporary shared_ptr which will not be successful if the external code that actually owns that memory has deleted it), this enforces case 2. Finally, shared_ptr is a reference counted pointer that allows shared ownership between multiple objects and when a shallow copy is performed on a shared_ptr, it will increase the reference counter and it will only delete once all objects that share the ownership are done with it. This solves case 3. An additional feature is that these are all automatic pointers (as in the deprecated auto_ptr), so they will delete the pointed-to memory (or decrease the reference counter) when they go out of scope. So essentially, it almost makes "delete" deprecated. Another very nice feature too, is that you can pack a custom deleter with these pointers if you require a special behavior in addition to the default "delete ptr;".

[maharjun]

@mike_2000_17:

thanks for that little information on the working of = operator in terms of the constructors. if im not mistaken thats what Fbody said in the first place so thanks to him too.

@Fbody:

see my actual class was a matrix where the int* pointer was intended to be used to store an array of integers. so, obviously, every time i assign a matrix to a new matrix there is no guarantee that the new matrix will have the same order as the current one and hence i would require resizing. That's why i put the delete in the first place.

1.) now, seeing as how assignment works. my most logical next step would be to remove the delete command from my constructor A::A(int) since this constructor is called only on creation of a new object(temporary or not) hence the pointer *p will always be a dangling pointer right?

now that ive done that, ill have to edit my copy constructor to perform a deep copy. that would imply that the values stored in the memory location pointed to by the temporary(or otherwise) object would need to be accordingly copied into the memory pointed to by my pointer.

but then again this would imply that whatever delete/assign statements that were in the constructor A::A(int) now get transferred to A::A(&A). so the same problems remain if only for a different constructor.

2.) Fbody earlier mentioned that overloaded operators should ideally only take in values by reference. but if thats the case, how can one pass a constant argument to them?. will the following code(just a guess) allow for it? id appreciate clarification on this matter.

void A::operator=(const int & a)
{
    int a = 3;//this should ideally cause an error i think because im attempting to
              //overwrite a constant;
}

Anyway, i think the only solution to my problem lies in overloading an assignment operator.

THANKS FOR ALL THE HELP TILL NOW. its really cleared a lot of my doubts.
p.s.- does the above q classify as a q that should be placed as a separate thread?

[Fbody]

>>see my actual class was a matrix where the int* pointer was intended to be used to store an array of integers
Is the array created as part of the class or outside the class with a pointer being given to the object? If the array is outside the class, you shouldn't worry about any of that stuff inside the class, that is the client code's responsibility. An object should only be conscious of itself and its members, not the outside world. If the array is actually within the class, then you must manage it within the class and keep the outside world oblivious. Looking at an object from the outside, the object should appear to be a "little black box". The outside world tells the object to do something, and it "magically" does it, without the outside world knowing how it did it. That is the whole point behind the encapsulation mechanism(s).

>>but then again this would imply that whatever delete/assign statements that were in the constructor A::A(int) now get transferred to A::A(&A). so the same problems remain if only for a different constructor.
No, it doesn't, don't forget, if a constructor is called, a new object is created, the pointer will have nothing attached to it because it's brand new. Simply attach an object to it and be done with it. A copy constructor creates a new object and makes it look the same as the object used to initialize the new object. An improperly written assignment operator is more of an issue than a copy constructor. You will need to do some extra memory management tasks there, but in general, a copy constructor and an assignment operator should be nearly identical. Put this code into your IDE and run it through your debugger. This will help you to see what actions occur when.

class A {
private:
  int * m_pInt;
public:
  A();                      //default constructor
  A(const int);             //specified constructor
  A(const A&);              //copy constructor
  ~A();                     //destructor

  A& operator=(const A&);   //assignment operator
};

//implement default constructor
A::A() : m_pInt(0) {}

//implement specified constructor
A::A(const int newInt) {
  m_pInt = new int(newInt);
}

//implement copy constructor
A::A(const A & other) {
  this->m_pInt = new int(*(other.m_pInt));
}

//implement destructor
A::~A() {
  delete m_pInt;
  m_pInt = 0;
}

//implement assignment operator
A &A::operator=(const A &other) {
  if (this != &other) {     //prevent assigning the object to itself
    delete m_pInt;
    m_pInt = new int(*(other.m_pInt));
  }
  return *this;
}

int main() {
  A defaultA;               //calls default constructor
  A specA1 = 5;             //implicitly calls specified constructor
  A specA2(10);             //explicitly calls specified constructor
  A copyA1 = specA1;        //implicitly calls copy constructor
  A copyA2(specA2);         //implicitly calls copy constructor

  defaultA = copyA2;        //calls assignment operator

  return 0;
}

>>overloaded operators should ideally only take in values by reference. but if thats the case, how can one pass a constant argument to them
Just like you did. Also, see Lines 22 and 33 above. They still pass as references, but the const doesn't allow them to be modified (without some inadvisable "tricky" programming).

>>does the above q classify as a q that should be placed as a separate thread?
Does it really matter? It's still related, isn't it???

[Narue]

>DOES PERFORMING DELETE OPERATION ON A DANGLING POINTER LEAD TO INSTABILITIES?
When you delete an indeterminate pointer, demons fly out of your nose and kill a kitten.

[maharjun]

>>Is the array created as part of the class or outside the class

it is a part of the class. and yes i do intend to completely hide it. but since i have to initialize it somehow, hence i need to take in an array as an argument right?(i use a really old version of turbo c++ so i don't really have the facility of vector's here).

2.)

//implement copy constructor

A::A(const A & other) {
    this->m_pInt = new int(*(other.m_pInt));
}

this works perfectly if m_pInt is required to store only a single integer.
but if it must store an array, it gets modified like this.(note that i have variables say int size that give an account of the size of the array pointed to by the pointer.

A::A(const A & other)
{
    //this->m_pInt = new int(*(other.m_pInt)); is of absolutely no use as it will store 
    //only the first element of the array;

    //this->m_pInt = other.m_pInt is of no use either since it would be a shallow copy.
    //hence the only remaining way is....

    m_pInt = new int [other.size];
    //write code to perform member-wise copy
    ...
    ...
}

note that i have made all my other constructors so as to fill in an array into the pointer m_pInt.

see i have already acknowledged that overloading the assignment operator is the only solution to my problem. this post is only for clarification reasons

>> >>but then again this would imply that whatever delete/assign statements that were in the constructor A::A(int) now get transferred to A::A(&A). so the same problems remain if only for a different constructor.

No, it doesn't, don't forget, if a constructor is called, a new object is created, the pointer will have nothing attached to it because it's brand new.

yes i observed that. ifact that was the reason i removed the delete from my original A::A(int) tag since the pointer was always 'fresh'.

however IF AN ASSIGNMENT OPERATOR is not defined and assignment(other than in declaration) is performed then if i have understood your's and mike_2000_17's post, the copy constructor is called. so the following code fragment will result in memory leak.

//prior to this two array's of integers arr1 and arr2 has been declared and initialized.
    A a1 = arr1,a2 = arr2;  //default constructor, perfectly healthy functioning.
    a1 = a2;                //MEMORY LEAK

to avoid this memory leak if i include a delete statement in the constructor then im gonna get the same deleting dangling pointers impropriety back in the program.
as follows.

A a1 = arr1; //delete performed on free pointer.

hence the only solution to this problem for me appears to be the assignment of an = operator thereby preventing calls to the copy constructor from intermediate assignments.

thanks for all the help though guys.

[mrnutty]

All of this could have been avoid by not using raw pointers. If you can, avoid pointers all together. If not, then use smart pointers.

[mike_2000_17]

>>however IF AN ASSIGNMENT OPERATOR is not defined and assignment(other than in declaration) is performed then if i have understood your's and mike_2000_17's post, the copy constructor is called. so the following code fragment will result in memory leak

Just to clarify: when the assignment operator is called, the copy-constructor is not called at all, but the compiler will produce a default assignment operator that essentially just assigns each data member of the Right Hand Side (LHS) to the Left Hand Side (RHS), i.e., a shallow-copy. So the code you posted there does not only lead to a memory leak, but makes both objects "a1" and "a2" share the same pointer (so, at delete time of both a1 and a2, the one that deletes first will be fine, but the second one to get deleted will "crash" (well it might not actually crash but there is an unacceptable chance that it will, especially if there are a lot of other operations between the two delete calls).