Should I add exception-handling to existing code base?

Question

I'd like to know the advantages and disadvantages of adding exception-handling to existing code.

I work on a SDK that controls h/w cards in a Windows environment.

The SDK is made of more than 100 DLLs that interact with each other. Our existing code base probably contains 100 000s (if not 1 000 000s) of lines of code. Our modules are also heavily multi-threaded.

We link with the proper library so that we use nothrow new (lic.lib instead of licp.lib).

Most of the code doesn't have exception handling. The code is written with that in mind.

int *p = new int[size];
if (p == NULL)
{
   // handle this case...
   // most probably return an error code
}

char *q = new char[size];
if (q == NULL)
{
    delete[] p;
   // handle this case...
   // most probably return an error code
}

We also use the RAII technique. For instance, we have a object created on the stack that automatically waits on and releases a critical section.

We want to improve the stability of our SDK. We were thinking of adding exception handling but I'm not convinced that it is the right way to improve the stability. I have to admit that I don't have much experience with EH.

The code, in general, checks for divide by 0 or checks for NULL pointers before dereferencing it. But, it still happens that such a case will happen. Since divide by zero or dereferencing a NULL pointer don't throw an exception, I am wondering how much useful is it to go thru 100 000s of lines of code and add exception handling which will change the workflow and may cause memory leaks if not handled properly. I experimented with SEH but I don't think it makes sense to start using SEH and it is Microsoft specific, isn't it?.

In my mind, I think that if it would be more useful to review the existing code and simply check for possible crashes such as divide by zero that may have been missed.

Also, if I were to add exception-handling, how would I proceed? Modify all the modules at once or start from the bottom-up (meaning, if Module A calls Module B which calls Module C, I would modify C, then B then A since we release our software quite frequently and we would probably only have time to modify C before the next release).

Thank you!

Answer 1

I'd like to know the advantages and disadvantages of adding exception-handling to existing code.

You don't say what you mean precisely by "exception handling", so I'll start with something fundamental: standard C++ (you tagged the question as c++) requires you to write code that "handles exceptions", for all but trivial applications, otherwise your code is faulty. Various parts of the C++ standard library are permitted to throw exceptions, including the new that your sample code uses. Therefore your code is already likely to have the possibility that exceptions could be thrown within it, which it must "handle". What happens in that case? Basically, you must write "exception safe code".

• It is an error for a program to leak resources in the face of exceptions. You use RAII so you should be OK.
• It is an error for any object to enter an inconsistent state after an exception is throw. Ensuring that can be much more tricky.

You should first focus on making your code exception safe.

Answer 2

With Legacy code, you should introduce exception handling in a few places as schedule permits; either the least accessed areas of the code (to reduce the risk of errors to the rest of the code base) or to where they would produce the most benefit (citical error places).

I do not recommend stalling a legacy project just to add exception handling everywhere. The hardest part about legacy code is to modify it and keep it working. After all, its been tested and its behavior is well documented.

Answer 3

I agree with Raedwald that if you are using C++ without a very careful coding standard to avoid EH (ex: using nothrow new, avoiding standard containers, etc), which I'm assuming the legacy code did not, then the code is already broken and is likely to leak and do sporadic things already in the face of exceptions it can already encounter like bad_alloc or bad_cast if you rely on dynamic_cast.

That said, from a very pragmatic standpoint with a legacy codebase, chances are that the legacy code managed to get away with it. And after all, how many non-trivial applications can gracefully recover from bad_alloc exceptions without very explicit control over memory allocation? Not many, and it doesn't cause the entire world to come to a screeching halt.

So I actually don't recommend rewriting the legacy code to try to catch exceptions and use RAII everywhere. You might utilize RAII here and there in code you absolutely have to modify, but I'd try to look for reasons not to change it too much. Write tests for it and try to stabilize it and turn it into a black box; a library of functionality to be used, not maintained and changed while wading through endless LOC indefinitely.

Now the main reason I pitched in here and resurrected this old thread (apologies!) is because of this comment:

The code, in general, checks for divide by 0 or checks for NULL pointers before dereferencing it. But, it still happens that such a case will happen. Since divide by zero or dereferencing a NULL pointer don't throw an exception [...]

In my strong opinion, you should not throw in response to things like null pointer access or divide by zero because those are programmer errors. Unless you're working in a mission-critical software where you want to gracefully recover even if the software is buggy in order to try to reduce the risk of costing lives or something like that, you don't want the application to gracefully recover in the event of programming mistakes. The reason you generally don't want to do that is because it has the downside of hiding bugs, making them silent, allowing users to ignore and work around them and maybe not even bother reporting them.

Instead for programmer mistakes you should generally favor assert which doesn't involve exceptions at all. If the assertion fails, the software in a debug build will come to a halt and typically display an error message telling you where the assertion failed down to the precise line of code. That is usually the quickest bet to detecting and fixing these programming mistakes when running a debugger in response to a bug report, so feel free to assert liberally.

Exceptions are most useful for external exceptional events outside of a programmer's control. An example would be reading a file that turns out to be corrupt. That's not within the programmer's control to handle so there it's appropriate to throw and recover. Another example is failing to connect to a server, a user jamming an abort button for an operation that is supposed to finish, etc. Those are exceptional external input events, not programmer mistakes.

The best way to recover from programmer mistakes like null pointer accesses and divide by zeros is to first detect them (assert is handy), write a test to reproduce it, fix it and get the test to pass, and call it a day, not throw exceptions and catch them while leaving those mistakes in there.