Victor Ruiz
Victor Ruiz

Reputation: 61

Memory Allocation and Try-Catch Block

I have a function to allocate a 2D array in order to not to expend more memory than I need:

_>

template <class Xvar> Xvar** New2 (unsigned int rows,unsigned int cols)
{
    Xvar**  mem;
    unsigned int size, i;

    size = rows * cols;
    mem = new Xvar* [rows];
    mem [0] = new Xvar [size];
    for (i=1;i<rows;i++)
        mem [i] = &mem [0][i*cols];
    return mem;
}

Now, I need to check if that memory is allocated. (handle memory allocation errors), without decreasing the performance of the function.

Should I use a try-catch block for each memory allocation, or only a unique try-catch block for the function.

template <class Xvar> Xvar** New2 (unsigned int rows,unsigned int cols)
{
    Xvar**  mem;
    unsigned int size, i;

    size = rows * cols;
    try {
    mem = new Xvar* [rows];
    }
    catch (...) { assert (...) } 
    try {
    mem [0] = new Xvar [size];
    } catch (...) { assert (...) }
    for (i=1;i<rows;i++)
        mem [i] = &mem [0][i*cols];
    return mem;
}

or something like:

template <class Xvar> Xvar** New2 (unsigned int rows,unsigned int cols)
{
    try { 
    Xvar**  mem;
    unsigned int size, i;

    size = rows * cols;
    mem = new Xvar* [rows];
    mem [0] = new Xvar [size];
    for (i=1;i<rows;i++)
        mem [i] = &mem [0][i*cols];
    return mem;
     }catch  (...) { assert (...) }
}

I think, the second way is not recommended because, if the first new fails, mem is NULL, so if we do mem [0] , we are accessing to a memory that is not allocated, so that application fails at that point, and error cannot be catched.

Upvotes: 0

Views: 2815

Answers (2)

Useless
Useless

Reputation: 67713

Don't catch the exception at all, just use RAII to clean up the memory when your function unwinds:

template <class Xvar> Xvar** New2 (unsigned int rows,unsigned int cols)
{
    unsigned int size = rows * cols;
    std::unique_ptr<Xvar*[]> mem(new Xvar* [rows]);
    mem[0] = new Xvar [size];
    for (i=1; i<rows; i++)
        mem[i] = &mem[0][i*cols];
    return mem.release();
}
  • if the first new fails, your function didn't do anything and hasn't leaked any memory: std::bad_alloc is thrown and that's it
  • if the second fails, the unique_ptr destructor takes care of that memory, so again you're ok.
  • the next three lines can't throw, so nothing can cause the second allocation to leak

As GmanNickG pointed out, this is still terrible code. It's the terrible code you asked for, but I don't want to give the impression I endorse the original design. I don't. It's terrible.

Once the caller has successfully acquired their Xvar**, the only way to dispose of it this:

int **ii = New2<int>(x, y);
...
delete [] ii[0];
delete [] ii;

which is brittle and unpleasant. Two better designs are:

  1. use a real matrix/2d-array template class which manages the storage, and can be returned by value. Then, the memory will be reclaimed when this value object goes out of scope

  2. use a smart pointer with a custom deleter

    template <typename T> struct 2dDelete
    {
        void operator() (T **rows) {
            delete [] rows[0];
            delete [] rows;
        }
    };
    
    template <typename T>
    std::unique_ptr<T*[], 2dDelete<T>> 2dNew (unsigned rows, unsigned cols)
    {
        unsigned size = rows * cols;
        std::unique_ptr<Xvar*[]> tmp(new Xvar* [rows]);
        tmp[0] = new Xvar [size];
        for (i=1; i<rows; i++)
            tmp[i] = &tmp[0][i*cols];
        // hand-over from the intermediate pointer (which only owned the
        // top-level row allocation) to the caller's pointer (which will
        // own, and clean up, the whole thing).
        return std::unique_ptr<T*[], 2dDelete<T>> arr(mem.release());
    }
    

Upvotes: 0

Lily Ballard
Lily Ballard

Reputation: 185653

In the second way, if the first new fails, then evaluation immediately jumps to the catch block and never even tries to access mem[0].

In any case, if you want to allow allocation to fail and detect this easily, you should probably use the nothrow variant, which simply returns NULL if the allocation fails. So something like

mem = new (nothrow) Xvar*[rows];
if (!mem) {
    // allocation failed, do whatever you want
}
mem[0] = new (nothrow) Xvar[size];
if (!mem[0]) {
    // allocation failed, do whatever you want
}

Upvotes: 4

Related Questions