C++ app: How to properly delete/release an allocated object?

Question

I am using a GCC compiler on Ubuntu 14.04.
I have a small communication app (not written by me) and I intend to embed it into my own app.
The small comm. app code looks like this:

UartComm * commInterface;   
...   
void commInit()   
{   
   commInterface = new UartComm;   
}   

// other functions here that use "commInterface"
// ...

int main()   
{
   commInit();   
   ...   
   if(smthing_1)
      return (-1);  
   if(smthing_2)
      return (-2);  
   ...
   if(smthing_n)
      return (-n);  
   //
   return (0);
}   

--
As seen above there is no delete commInterface; in the original code so, if I embed the above code into my app, rename main() to commFunction() and call it many times, I will have a lot of memory that is not de-allocated.
The above code is a simplified version. In fact, the code has many exit points/returns. It also has some functions that throw exceptions (I'm not 100% sure they're all catched and handled properly).
So, I guess adding delete commInterface; before all returns will not be enough in this case...
Hence, my question: Is there a way to properly delete/release commInterface in order to embed and use the above module without worrying about memory leaks? Smart pointer maybe or some other idea...?
Two remarks:
1) I've enabled C++11;
2) I' not using (and don't wanna use) boost.
Thanks in advance for your time and patience.

Answer 1

In my hummble opinion this is a scheme that global variables are to be avoided. Use smart pointers to accomplish RAII and in particular std::unique_ptr in the following manner:

int commFunction() {
   auto commInterface = std::make_unique<UartComm>(); 
   ...   
   if(smthing_1)
      return (-1);  
   if(smthing_2)
      return (-2);  
   ...
   if(smthing_n)
      return (-n);  
   //
   return (0);
}

Mind however that std::make_unique is C++14, alternatively for C++11 use the following:

std::unique_ptr<UartComm> commInterface(new UartComm);

Mind also, that using the suggested scheme a new UartComm object is going to be created every time commFunction is called. If creation of UartComm is an expensive operation or if your design requires reuse of a single UartComm object, you could use the singleton pattern for your UartComm object.

Answer 2

Is there a way to properly delete/release commInterface in order to embed and use the above module without worrying about memory leaks? Smart pointer maybe or some other idea...?

Here is another idea: use a static instance. It will be constructed on first use, and destroyed at the end of the program. This will be faster than dynamic allocation too (though, it doesn't matter much for a single allocation).

UartComm& commInterface() {
   static UartComm interface;
   return interface;
}

Answer 3

I suppose that commInterface is a singleton to be provided to other parts of the application. I suppose further, that you want to make sure that the singleton objet is initialized whenever it shall be used in the application.

Then, I'd suggest to make the variable static in order to not expose it to other translation units (which might then access it in an uninitilized state); Instead, provide an accessor function like UartComm *getCommInterface (). Before accessing the variable, check if it is initialized. See code below as an example:

static UartComm* commInterface = nullptr;

void commInit()
{
    if (!commInterface)
        commInterface = new UartComm;
}

UartComm *getCommInterface () {
    commInit();
    return commInterface;
}

Note that, as you program in C++, you should also think of encapsulating this singleton mechanism in some class; but this is a different topic then.

Answer 4

1. Delete commInterface; that's all!:) Delete it all in the end of program.

2. Why do many objects. I was developing application for RS-232 port (COM-port) and it have 1 object "RS-232" for communication. Or you have Ethernet connection?

Answer 5

The general rule is that a dynamically allocated object must be released. And that each form of allocation requires the corresponding deallocation.

Although modern operating systems will often clean up for you as the program ends (no need to use operator delete), it is a BAD habit to rely on this - not all operating systems make that guarantee. And operating systems do have bugs. Furthermore, there is your situation - if you then find you want to rename main() to something else to reuse it, you have a problem of a memory leak - your problem would have been easily avoided if you cleaned up in code, rather than relying on cleanup as the program exits.

In your case, the allocation is

commInterface = new UartComm;   

so the corresponding deallocation is

delete some_pointer;

where some_pointer might be commInterface (assuming it is visible) or another pointer, of the same type, which stores the same value.

In general terms, there are many other ways to ensure the object is correctly released. For example;

• Release it in the same function that allocates it. The problem is that prevents the object from being used after the function returns.

• Return the pointer to the caller, and the caller releases it.

  UartComm  *commInit()   
  {   
       UartComm *commInterface = new UartComm;   
         // whatever
  
       return commInterface;
  }   
  
  int main()
  {
        UartComm *x = commInit();
  
            // whatever
  
         delete x;
  }
  

• Like the above, but use a smart pointer, so it is not necessary to manually deallocate (the smart pointer does that).

  std::unique_pointer<UartComm> commInit()   
  {   
       std::unique_pointer<UartComm> commInterface(new UartComm);   
  
         // whatever
  
       return commInterface;
  }   
  
  int main()
  {
        std::unique_pointer<UartComm> x = commInit();
  
            // whatever
  
        //   x will be cleaned up as main() returns - no need to release
  }
  

There are, of course, numerous other options. For example, don't use dynamic allocation at all, and simply return an object by value.

Answer 6

If you can remove the global variable I'd go with the singleton solution presented by user2079303 or the local unique_ptr version of 101010. Which one depends on wether you want to recreate the interfaces every time you call your commFunction() or not.

If removing/changing the global variable is not possible I see two possibilities with minimal code changes (again depending on whether you want to recreate the object or not):

UartComm * commInterface = nullptr; //nullptr not necessary but makes it more explicit
...
void commInit()
{
    //only create commInterface the first time commInit() is called
    if( !commInterface )
        commInterface = new UartComm;
}

or

UartComm * commInterface = nullptr; //nullptr not necessary but makes it more explicit
...
void commInit()
{
    //destruct old UartComm and create new one each time commInit is called
    delete commInterface;
    commInterface = new UartComm;
}

Again, those are not solutions I'd recommend in general, but under the previously mentioned constraints they are probably the best you can do.

Also keep in mind that none of these solutions is thread safe (even if UartComm was).