Tuesday, June 17, 2008

Bad Idiom: Using Strings on Stack

Here's what I've seen recently:
#include <stdexcept>

Class::Class(int arg)
{
...
if(/* error */) {
// original code
//throw std::runtime_error("Class::Class: Wrong argument!");

// later addition
char str[100];
sprintf(str, "Class::Class: argument %d is too big", arg);
throw std::runtime_error(str);
}
...
}
int main()
{
try { c = new Class(100000); } // say it's allocating memory
catch(std::runtime_error& e) {
printf("Got an error: %s\n", e.what());
}
}
The code sins in many ways:
1. the catch() is wrong as the code does not throw a reference but an object!
2. the throw() is poisoned: it throws a string from the stack which will go out of scope when the constructor is exited; the application won't crash immediately [the stack pages are not unmapped];
3. text of the exception will be junk (and may not have a \0 terminator) and attempting to print it may crash the app in funny and random ways.

The person who butchered the code and used the class hierarchy suffers from the cargo cult programming syndrome because:
1. did not pay attention to original code;
2. did not bother to read the Doxygen documentation of the original classes;
3. does not understand the life cycle of an auto var allocated on stack;
4. does not analyse all the implications of using and changing other people's code.

-ulianov