Reputation: 4320
I am using STL containers in my code (being developed in C++ using Visual Studio 2010)
I have never used exception handling with STL containers before. Since STL containers throw bad_alloc
exception, I plan to use like in sample code shown below. Let's assume function()
gets called in low memory situation.
Now, I am not sure if it is full proof code OR do I need to do any additional cleanup activity.
class MyClass
{
std::vector<int>* integer_vector;
public:
MyClass()
{
std::string message;
message += "my message"; // bad_alloc could be thrown from here
integer_vector = new std::vector<int>; // bad_alloc could be thrown from here
}
};
void function()
{
try
{
MyClass* myclass_ptr;
myclass_ptr = new (std::nothrow) MyClass;
if (myclass_ptr==NULL)
{
// ANY CLEANUP NEEDED HERE ?
return;
}
std::map<int, char> myintcharmap; // bad_alloc could be thrown from here
}
catch(...)
{
// ANY CLEANUP NEEDED HERE ?
return;
}
}
Please can someone have a look and help.
Upvotes: 0
Views: 405
Reputation: 15905
You have two main potential leaks in the code you show. Both of which arguably stem from using raw pointers. You should prefer using std::unique_ptr
(if you have C++11) or other similar "smart" pointers to indicate ownership, and for exception safety in general. Modern guidance is to avoid almost all usage of new
or delete
; when they cannot be avoided, they need to be paired. Note that your code has two calls to new
but none to delete
.
Inside function
, the core problem is you could fully allocate data "owned" by myclass_ptr
, cause an exception in later allocations, and then not be able to clean it up as myclass_ptr
is no longer in scope.
Let's say you fixed that so it cleaned up the MyClass
instance if an exception occurred after its creation. Your code would still leak because inside MyClass
there's currently a similar problem with integer_vector
. Although you could follow the rule of three and write a destructor to handle this case, it's probably easier to use a smart pointer here as well.
Exception handling is much bigger, much more opinionated topic. I'll leave it with the summary that it's typically bad to catch exceptions and squash them (usually that's only legit in an outer loop of a program that needs specific kinds of stability). It's also typically bad to catch exceptions in a scope so narrow that you don't know how to handle them. (For example, how would function
decide whether to try again, give up, or use another approach? Its caller, or the caller further on up the chain, may have more information and be in a better place to handle this.)
Upvotes: 2
Reputation: 23793
In most cases you should not deal with bad_alloc exceptions. Your try/catch
should be removed, as well as if (myclass_ptr==NULL)
.
Ask yourself : if the process memory is exhausted, what could I possibly do ? Best thing you can hope is to log something, clean up / free the system resources, and let the program terminates. This is the only right thing to do.
You can do this by setting the new_handler (with set_new_handler). It will be called by the new operator if memory allocation failed.
std::set_new_handler(my_handler); // do your cleanup in 'my_handler'
Upvotes: 2