Re: Memory Leaks - Can you help me find them in ths snippet

From:

"Alf P. Steinbach" <alfps@start.no>

Newsgroups:

comp.lang.c++

Date:

Mon, 28 Jan 2008 17:23:33 +0100

Message-ID:

<13ps08e4pvvpp0b@corp.supernews.com>

* nmehring@gmail.com:

I am writing some c++ code that interacts with a native C library and
have to do some dynamic memory allocation to support it. I am getting
memory leaks and I think this piece of code is the culprit. Can
anyone tell me what I might be doing wrong?

char **columns;
columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));

if (NULL == columns) return FALSE; //memory allocation failed

//allocate space for the column names in the char**
int column_index;
for (column_index = 0; column_index < lColumnCount; column_index++)
{
// Allocate for the maximum owner.table.column notation
columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);

if (NULL == columns[column_index])
{
delete[] columns; //memory allocation failed
return FALSE;
}
}

//put the data in the char**
for (column_index = 0; column_index < lColumnCount; column_index++)
{
strcpy (columns[column_index], CStringColumnName[column_index]);
}

//do some stuff with columns
.......

delete[] columns;

Using malloc for allocation and delete[] for deallocation is Undefined
Behavior.

Code below would be better expressed using ScopeGuard.

But in order not to bring in that library (disclaimer: code not touched
by compiler's hands):

   class Columns
   {
   private:
       std::vector<char*> myColumns;

       void dealloc()
       {
           for( size_t i = 0; i < myColumns.size(); ++i )
           {
               delete[] myColumns[i];
           }
       }

   public:
       Columns(): myColumns( ::lColumnCount )
       {
           try
           {
               for( int i = 0; i < lColumnCount; ++i )
               {
                   myColumns[i] = new char[SE_QUALIFIED_COLUMN_LEN];
                   strcpy( myColumns[i], ::CStringColumnsName[i] );
               }
           }
           catch( std::bad_alloc const& )
           {
               dealloc();
               throw;
           }
       }

       ~Columns() { dealloc(); }

       char** pFirst() { return &myColumns[0]; }
   };

   bool foo()
   {
       try
       {

           Columns columns;
           someApiFunc( columns.pFirst() );
           return true;
       }
       catch( ... )
       {
           return false;
       }
   }

foo() could be simpler if it signalled failure via exception rather than
via a bool result.

Cheers & hth.,

- Alf

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?