One of the biggest hassles in programming is handling error conditions. It’s also one of the most important parts to get right because improperly handled errors lead to security defects and general unhappiness of your users.
Error-handling code sucks because it complicates the flow of the code that does real work. Consequently, most programming articles leave it as an exercise to the reader. This often has tragic consequences as many programmers leave error-handling as an exercise to end-users. sigh
Here is some typical code which handles errors, but poorly. Lots of projects I “ported” early in my career contained similar code. Note: Porting is the process of converting a game or program which runs on one platform (like the PlayStation) to another (Windows).
/* Duplicated error-handling code; not recommended */
char * LoadFile_duplicated_code (char * fname) {
int length;
char * data;
FILE * file;
if (NULL == (file = fopen(fname, "rb")))
return NULL;
if (fseek(file, 0, SEEK_END)) {
fclose(file);
return NULL;
}
if (-1 == (length = ftell(file))) {
fclose(file);
return NULL;
}
if (NULL == (data = (char *) malloc(length + 1))) {
fclose(file);
return NULL;
}
if (fseek(file, 0, SEEK_SET)) {
free(data);
fclose(file);
return NULL;
}
if (length != fread(data, 1, length, file)) {
free(data);
fclose(file);
return NULL;
}
// Cleanup
fclose(file);
// NULL terminate the result
data[length] = 0;
// Success!
return data;
}
The code above isn’t terrible, inasmuch as it handles all error conditions. I’ve seen much worse code that didn’t check for errors at all!
Here’s another solution that uses goto statements. I wrote code in this style early in my career; if Warcraft and its tools are ever open-sourced I expect you’ll see code just like this:
/* cleanup using gotos; not recommended */
char * LoadFile_goto_cleanup (char * fname) {
int length;
char * data;
FILE * file;
if (NULL == (file = fopen(fname, "rb")))
return NULL;
if (fseek(file, 0, SEEK_END))
goto err1;
if (-1 == (length = ftell(file)))
goto err1;
if (NULL == (data = (char *) malloc(length + 1)))
goto err1;
if (fseek(file, 0, SEEK_SET))
goto err2;
if (length != fread(data, 1, length, file))
goto err2;
// Cleanup
fclose(file);
// NULL terminate the result
data[length] = 0;
// Success!
return data;
err2:
free(data);
err1:
fclose(file);
return NULL;
}
Using gotos for cleanup is effective but is viewed as anathema by many programmers due to Edsger Dijkstra’s famous dictum, “Goto Considered Harmful”. I used this style for several years but eventually came up with a solution I like better: the forever-loop pattern.
/* centralize cleanup code: recommended */
char * LoadFile_central_cleanup (char * fname) {
int length;
char * data = NULL;
FILE * file = NULL;
char * result = NULL;
for (;;) {
if (NULL == (file = fopen(fname, "rb")))
break;
if (fseek(file, 0, SEEK_END))
break;
if (-1 == (length = ftell(file)))
break;
if (NULL == (data = (char *) malloc(length + 1)))
break;
if (fseek(file, 0, SEEK_SET))
break;
if (length != fread(data, 1, length, file))
break;
// NULL terminate the result
data[length] = 0;
// Success!
result = data;
data = NULL;
break;
}
if (data) free(data);
if (file) fclose(file);
return result;
}
The chief advantage is that the error-handling code doesn’t obscure the real work of the function. Further, the cleanup code is in the main path so it gets executed each time the function runs and is therefore more likely to be correct, always a bonus since most error code isn’t thoroughly tested.
I hope this missive helps improve your code-fu. Good luck!
About Patrick Wyatt
Having written similar code (and feeling moderately slimy and guilty for it), I am a big fan of this pattern. Do you think it would be worthwhile to replace your loop with a do { … } while (0) so that you don’t need to have a final break statement, and to protect against having an accidental forever loop?
The do { … } while (0) pattern would also work, and it is a better pattern because, as you mention, there’s no risk of looping forever (which I accidentally did once — but not in production code).
I chose not to use do/while because Visual Studio 6, which I used for almost 10 years, would spit out a warning message about that construct. It was possible to override the warning with a #pragma, but I ended up sticking with the for (;;) pattern for that single reason.
It’s possible to eliminate the warning by coding as following :
do {…} while (__LINE__ == -1);
I like this solution better, since it guarantees that no looping will ever occur :)
I’ve got to say that I find this really ugly. Adding a loop that doesn’t do any looping makes the code less, not more readable, and adds potential for bizarro errors if you forget to terminate it correctly. “goto” may feel icky, but in this case it reflects the semantics of what is going on, and used idiomatically isn’t going to turn your code into spaghetti.
Also, the “goto” approach lets you specify more complex cleanup conditions that can’t be expressed as a series of “if (resource_exists) { free_resources(); }” clauses, for example, releasing a semaphore.
At ArenaNet some devs had the same concerns you’ve mentioned. Mike O’Brien, one of my partners at ArenaNet (and one of the best programmers I’ve ever worked with) created a macro called “ONCE” which declared a unique variable for the for-loop. It worked like this:
#define _LINENAME_CONCAT( _name_, _line_ ) _name_##_line_
#define _LINENAME(_name_, _line_) _LINENAME_CONCAT(_name_,_line_)
#define _UNIQUE_VAR(_name_) _LINENAME(_name_,__LINE__)
#define ONCE bool _UNIQUE_VAR(once) = true;
_UNIQUE_VAR(once);
_UNIQUE_VAR(once) = false
(_UNIQUE_VAR cribbed from http://spiralstormgames.com/blog/iphone-development/creating-unique-variables-within-your-preprocessor-macros/)
Then the loop was defined like this:
for (ONCE) {
…
}
This did a much better job of documenting the intent of the loop, and preventing the infinite loop problem.
However… Visual Studio 6 did a poor job of turning this into optimal code in DEBUG mode. I understand why: the compiler writers are not trying to optimize in DEBUG; they’re trying to maintain the original intent of the code for easy debugging. In any event, this was a drag because most of our backend production code was compiled for DEBUG mode; only the file server (FileSrv.dll), game server (GwSrv.dll) and game client (Gw.exe) were compiled for RELEASE. Since I wrote a lot of server code I stuck with for(;;) instead of for(ONCE).
The loop method is great for C code. I don’t find it any less readable; it’s fairly obvious what it’s purpose is. But I would only use it in a language lacking the concept of exceptions.
In C++ I usually create generic wrappers for libraries whose functions return error codes in order to make them throw exceptions instead. It’s then possible to do error handling in catch blocks or RAII objects to simulate finally blocks. Not only is this less work to use afterwards, its also impossible to forget to verify an error code.
The D programming language solves this problem quite elegantly using the scope keyword:
auto file = fopen(fname, “rb”);
if(!file) return;
scope(exit) fclose(file);
// …
scope can take one of exit, success and failure to execute the block always, on normal exit and on exceptional exit respectively.
Jeremie — I think D has one of the nicest solutions to handling cleanup, thanks for sharing the syntax example.
Many game devs are used to working on platforms that are notoriously short of RAM compared to PC/Mac/Linux computers, so we generally avoid C++ exception-handling because of the runtime cost in object-code size. There are other reasons too, but it’s a religious issue among programmers that each side feels so strongly about that I doubt any language will ever be able to make everyone happy.
That’s a very good point, but does it still holds true for today’s consoles? UnrealEngine3 for example makes extensive use of exceptions, at the expense of swarming the code with fallbacks to returning error codes, most likely for the reasons you mentioned.
I never did assembly on consoles therefore I don’t know the actual cost of their exception handling. However, on Windows setting a frame handler takes two instructions in the prolog (push, mov) and one more in the epilog (pop). Compiler optimizations remove most, if not all, if the remaining overhead. Linux and OSX are roughly similar, using posix signals instead of SEH. In terms of performance exceptions are only expensive during the stack unwind process following a call to throw().
Exceptions are also friendly to functional programming; in D throwing exceptions does not break function purity. It’s not possible to compose functions or pass them to generic algorithms when their result is given through an out parameter. Quite a lot of development time can be saved over writing, debugging and maintaining return codes.
Exceptions also only require code in frames interested in handling exceptional cases and the frame throwing the error. The CPU also saves itself a lot of branch predictions since there are no more error check conditions on every function call. The resulting code is often faster, much like using a GC is often faster than manual memory management.
There is however a small overhead in object-code size as you said, especially since exceptions require RTTI to be enabled. But it usually get dwarfed by having more productivity and more performance. RTTI is also needed for the runtime features of C++ such as dynamic casts and cannot be removed in D.
While I agree that we can forget exceptions for SNES and N64 programming, I’m pretty sure current consoles can handle the small increase in object-code size, which may end up a decrease after the compiler is done optimizing. It’s fair to assume object-code size will be a non-issue for next-gen consoles as it is for PCs today.
Whether to use exceptions or not is a highly contentious issue; there is no right answer for every project.
Depending upon the type of game you’re writing, and what platform you’re developing for, the runtime overhead (extra code-size, slower execution) of exceptions may not be so large as to preclude their use.
I will let you know which side of the line I’m on for writing performant game-client applications: I wouldn’t use exceptions. If I were writing a current-gen console game (360/PS3) I wouldn’t want to spend any extra bytes of RAM when there’s already so little. While Warcraft 1 (barely) fit in 4MB with some config.sys tweaking, and Warcraft 2 fit in 8 MB, the 512 MB RAM on the current generation of consoles is tiny for top-tier games when taking into account textures, vertex buffers, animations, streaming audio, game code, network buffers, and so forth.
Interesting concerns. Did you ever do real measurements that showed how much RAM you saved by turning off exceptions? How much % in binary code size would you expect to save?
I wrote a detailed post in answer to your question, which got eaten by the Internet. *sigh*. Here is my second attempt.
In order to fairly compare costs it would be necessary to write a program using exceptions and then rewrite using C-style error-handling. Based on one vendor’s experience (an admittedly small sample), there was a 15% code-space overhead for exception handling (from http://www.open-std.org/jtc1/sc22/wg21/docs/TR18015.pdf)
If you understand how exception handling is implemented you’ll have a great understanding of why those costs exist. Here are some good references:
http://www.hexblog.com/wp-content/uploads/2012/06/Recon-2012-Skochinsky-Compiler-Internals.pdf
http://www.codeproject.com/Articles/2126/How-a-C-compiler-implements-exception-handling
http://www.microsoft.com/msj/0197/exception/exception.aspx
Thanks Patrick for taking the time to dig out these infos. From taking a glimpse at the paper, it would seems that the 15% overhead is a rather pessimistic value (“… as this vendor had no need to optimize for space.”) and also there’s no numbers wrt. to the space overhead of the code-based approach. Ah! the joy of performance questions. :-)
http://ideone.com/kwmzJ
If a situation allows to use C++11, then some utility function with lambda as above link can be quite handful.
You could also use try/finally (even without expecting any exceptions) or just move the code that does the reading into a method that can just return on failure.
While I agree that centralised error-handling is important, forever loops are no more or less dangerous than a well-used goto in my opinion, and are less legible (since it’s a for, but you’re not iterating). By rights C should have some sort of jump statement that break out of the current block, even if that block is not a for, a while or a switch. The inability to do so is the reason why the forever loop needs to be used: we get a “break statement not within loop or switch”compile-error otherwise.
Using a single-case switch would at least prevent us from every having an eternal loop, but would be no more legible/intuitive. What’s more variable declarations are illegal within switches…
switch(0) { default:
if(err1) break;
if(err2) break;
if(err3) break;
}
// cleanup
All told I think what we really need is a way of breaking out of anonymous blocks, but failing that goto statements probably make for the most legible code.
Hmm … maybe instead of “Code of Honor”, this Blog (at least the few C++ posts I read) should be called “Tales from the Real World” :-)