How to struct cleanup code?

Question

We are developing application in a platform whose compiler doesn't support exception in C++ language.

When we call API we need to check the error code from the return result. Sometimes we need to do some cleanup when error happen. We define an macro CHECK_ERROR_GOTO to check error code. So the code like this.

int res = callAPI1();
CHECK_ERROR_GOTO(res, cleanup);
A* object1 = callAPI2();    // HERE WE WILL GET ERROR, 
                             //BECAUSE GOTO SKIP INITIALIZATION OF OBJECT1
CHECK_ERROR_GOTO(object1 == NULL, cleanup);
C* object2 = callAPI3(object2);
CHECK_ERROR_GOTO(object2 == NULL, cleanup)
return 0;
cleanup:
    //Cleanup code here.
    delete object1
    delete object2

As the code show, we will get object1 initialized skipped by goto, so we need put object1 and object2 in the header of function, which is bad because we should put it in the place we need to use. And add {} to create local scope doesn't work, because the cleanup code need to using the local variable.

So anyway we can arrange the code, so we doesn't need put variable initialize in the begin of function?

Answer 1

You say your compiler doesn't support exceptions. But does it support executing destructors on early return? I sure hope it does. You should make sure with a simple test.

Which means you can just use smart pointers and other classes that release resources in the destructor.

int res = callAPI1();
if (!res) return -1;
scoped_ptr<A> object1(callAPI2());
if (!object1) return -1;
scoped_ptr<C> object2(callAPI3(object1.get()));
if ((!object2) return -1;
return 0;

scoped_ptr here could be any suitable smart pointer class: your compiler's unique_ptr in the unlikely case is supports it, auto_ptr if you're feeling brave, boost::scoped_ptr if you can use Boost, or just write your own scoped_ptr, it's pretty trivial.

template <typename T>
class scoped_ptr {
  T* raw;

  struct bool_dummy { void fn() {} };
  typedef void (bool_dummy::*pseudo_bool)();

  scoped_ptr(const scoped_ptr&); // non-copyable
  scoped_ptr& operator =(const scoped_ptr&);

public:
  scoped_ptr(T* raw) : raw(raw) {}
  ~scoped_ptr() { delete raw; }

  T* get() const { return raw; }
  T& operator *() const { return *get(); }
  T* operator ->() const { return get(); }
  operator pseudo_bool() const { return get() ? &bool_dummy::fn : 0; }
  bool operator !() const { return !get(); }
};

Answer 2

Right, not the most beatiful code but it will do:

int res = callAPI1();
if (!res) /* inverse of CHECK_ERROR_GOTO(res, cleanup); ??? */
{
    A * object1 = callAPI2();

    if (object1 != null) /* inverse of CHECK_ERROR_GOTO(object1 == NULL, cleanup); */
    {
        C * object2 = callAPI3(object1); //I suppose you want object1 here and not 2

        if (object2 != null) /* inverse of CHECK_ERROR_GOTO(object2 == NULL, cleanup);*/
        {
            /* if we reached here then both object1 and 2 are created
               thus delete both before returning to not get any memory leaks */
            delete object1;
            delete object2;

            return 0;
        }

        /* if we reached here then the creation of object2 has failed.
           Only delete object1 because object2 is not initialized. */
        delete object1;

        /* do some kind of return here? like -3 */
    }


    /* do some kind of return here? like -2 */
}

/* do some kind of return here? like -1*/

Answer 3

I like to do it like this:

int foo()
{
  A* object1 = 0; // make sure you initialize those two
  C* object2 = 0; // so that further delete calls always work

  for (;;)
  {
    int res = callAPI1();
    if (res == -1)
      break;

    object1 = callAPI2();
    if (object1 == 0)
      break;

    object2 = callAPI3(object1);
    if (object2 == 0)
      break;

    return 0;
  }

  delete object1;
  delete object2;

  return -1;
}

However, it's not always possible to articulate the code like above. But when it is, I find it great. It has the advantage of placing (just like a goto) the cleanup code in a single place. Otherwise it could become unmaintainable like:

// don't do this:
if(object2==0)
{
  delete object1;
  return 0;
}
// ... more code ...
if(object3==0)
{
  delete object2;
  delete object1;
  return 0;
}
// ... more code ...
if(object4==0)
{
  delete object3
  delete object2;
  delete object1;
  return 0;
}