What could this curious combination of "while" and "delete" mean?

Question

Reviewing a quite old project I found the following curious code snippet (only relevant code extracted):

class CCuriousClass {
    ~CCuriousClass();
    CSomeType* object;
};

CCuriousClass::~CCuriousClass()
{
    while( object != NULL ) {
        delete object;
    }
}

Have I overseen anything or is it a plain road to undefined behaviour?

What I see here is that if object is a null pointer at the point of CCuriousClass::~CCuriousClass() being called everything will be fine - no action taken - but if object is not null this will be an infinite loop with undefined behaviour inside.

Is this most likely a bug or some smart construct I don't understand?

Answer 1

This looks like a bug.

Answer 2

The Code seems buggy . Make sure that destructor should not throw any exceptions otherwise it may terminate the program and it should be like.

CCuriousClass::~CCuriousClass()
{
    try
    {    
          if( object != NULL ) 
          {
              delete object;
              object = NULL;
          }    
    }
    catch(...) 
    {  }
}

Answer 3

Since your question seems to imply "What could somebody have meant with this?" and not "Why is this a fantastic idea?" I suggest the following:

class CSomeType {
    CCuriousClass* m_plistentry;
    CSomeType* m_pnext;

    ~CSomeType() {
        m_plistentry->object = m_pnext;
    }
};

The basic idea could have been that the owner points to the head of a list, and the list can only be deleted at the head. If the head gets deleted, it sets its parent pointer to the new head of the list. If the parent is destructed, it destroys every list element.

Now this is clearly code from crazy town.

Answer 4

As you say, it's a bug. delete does not set the pointer it deletes to NULL, so what you have is an infinite loop, which may or may not be terminated by the undefined behaviour you get from deleting the same pointer twice.

Answer 5

It seems like a bug, unless CSomeType's destructor can somehow modify this object.

Answer 6

not only is it probably a bug but it is an absolutely pointless check as deleting a null pointer is fine. replace with

CCuriousClass::~CCuriousClass()
{    
    delete object;    
}

or better yet use a smart pointer and get rid of the destructor altogether.

Answer 7

It could be that some lunatic has implemented CSomeType with a back-reference to its owning CCuriousClass, and its destructor sometimes creates a replacement. Something like this:

class CSomeType
{
public:
    explicit CSomeType(CCuriousClass &parent) : parent(parent) {}
    ~CSomeType()
    {
        parent.object = respawn() ? new CSomeType(parent) : 0;
    }
private:
    CCuriousClass &parent;
};

I'm not suggesting that anyone should ever write such twisted logic. It probably still gives undefined behaviour, since I believe delete is allowed to modify the pointer. But it might explain why someone might think the given code might be valid.

On the other hand, it's probably just a bug caused by a misunderstanding of how delete works.

Answer 8

This behavior is possible, provided an instance of CSomeType knows the address where a pointer to itself is stored (CSomeType** member in CSomeType) so that it can reset it on deletion. I have no idea why it could be needed.

example:

struct self_know{
    self_know** pptr;
    int cnt;

    static self_know* create(self_know **_pptr){
        *_pptr = ::new self_know;
        (*_pptr)->cnt = 10;
        (*_pptr)->pptr = _pptr;
        return *_pptr;
    }

    void operator delete(void*it){
       self_know *s = (self_know*)it;
       if(--s->cnt<0){
         *(s->pptr)=0;
         ::delete s;
       }

    }
};

#include<iostream>
main(){
  self_know *p = 0;
  self_know::create(&p);
  while( p != 0){
     std::cout << p->cnt << std::endl;
     delete p;
  }
}

Answer 9

Check out the CSomeType class definition. There may be a "!=" function overloading around. Otherwise it's clearly a bug.

Answer 10

One other possible theory is that someone played a nasty trick with preprocessor. Say:

struct delete_and_null {
    template<class T>
    delete_and_null& operator, (T*& p) {
      delete p;
      p = 0;
      return *this;
    }
} delete_and_null;

#define delete delete_and_null,

That doesn't explain the need for the loop, but at least it would then avoid U.B. and terminate eventually.

Answer 11

I suspect it's from a crazy C programmer who doesn't understand destructors and/or operator delete. Either that or someone who clumsily mistyped a 'while' instead of an 'if' and didn't know that delete already checks for null.

I wouldn't waste time speculating on lunatic code. Important thing is to recognize that it's silly and crazy.