Reputation: 61
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
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();
}
new
fails, your function didn't do anything and hasn't leaked any memory: std::bad_alloc
is thrown and that's itunique_ptr
destructor takes care of that memory, so again you're ok.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:
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
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
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