Working on a web server in C++, (I really was working on an adaptive thread pool class, but there's a lot of stuff to test web servers for scalability, so it was easy enough to write a minimal web server), I'd wrapped sockets that talked to clients in a class that would let me insert things exactly as you would on a std::ostream.
There's one problem with inserters though, they can't return error codes.
They have a signature something like
They always return their first argument so that you can chain, something like
So what do if there are errors? Writing data into a socket connected to a browser often fails. If the user clicks onto another page, the socket isn't connected to anything anymore.
One choice is to keep state about validity in the sockfdwrapper, and have every inserter check to see if the sockfdwrapper is still valid. If it's not, you'd just return the first argument.
That's kind of a pain though. It is a great idea to keep validity information in the wrapper, and I do that, but it doesn't fit well with inserters. Instead, I decided to throw an exception when something went wrong.
I figured it would be easy enough to make an exception derived from std::exception. It would take errno as an argument when something went wrong and pass information back to anyone who wanted to catch it. The standard what() method would let them ask for information.
A std::stringstream lets you insert anything into it, and then use the str() method to get a std::string back. The std::string has a method, c_str() which returns a reference to a regular null terminated string which we need for the return value of what(). The signature of what(), virtual const char* what() const throw() is from the parent class, std::exception.
So I'd been testing scalability, and fixed some bugs and decided to run valgrind against the server. valgrind is a tool that is used for, among other things, to find out about memory leaks. I got this:
The thing to notice is that we made an invalid read, it was by basic_ostream, so we're trying to write something, and that something was previously freed when a std::string was destroyed. It was freed by line 365 in httpserver.cpp. That line is the one shown in the catch block below that tries to insert the return value of sif.what() into std::cerr.
So what happened? The problem is in the return value from what(),
The signature of stringstream::str() is:
so it returns a string which will be local to our what() method. Then we call c_str() on the string, which has signature:
i.e. it returns a pointer into a data area of the string. I'm sure you can see it now, what() returns the pointer into a data area of the string, but the string is destroyed when it goes out of scope when what() returns! If C++ was a garbage collected language, there'd still be a reference and we'd be ok, but it's not.
This is viable. We give them a way to get the original errno number, and they could generate the string themselves within their own local scope. It's a shame though that every place that does this has to in essence duplicate the code we'd remove.
If we did that, what() couldn't set the data to anything, because its signature declares that what() is const, i.e. it won't change the class in any way.
If we remove the const from the declaration of what, then, since that is part of the signature, what() won't be recognized as being a replacement for exception's what, so catch blocks that catch exception will not get the output of our what(), but of exception's what. If we remove the const, we might as well remove the virtual also. While we're at it, we could just make our what return a string() if we wanted.
Perversely, catching as a const socket_insert_fail& will fail, since we will call a non-const method.
That would be ok, it we want to make it a requirement that the catch block explicitly catches our class, they have to do that to get the error number anyway. So, this would be viable.
One good thing about doing it this way, is that resources might by freed before what() is called so we're less likely to have other problems that would lead to an exception in our what().
This is also a great idea, with the caveat that we are more likely to throw an alloc exception when we create the string if resources are getting tight. Our main reason for throwing this will be because the other end of the communication has closed their end of the socket, but it can also happen if memory has gotten tight. Another disadvantage of this is that each exception has to pay the penalty of string construction even if the client doesn't use it.
I'm going with #3! Here's the new definition:
Now we create a temporary stringstream in the constructor, and assign a copy of its string to the_str. The lifetime of the_str is the same as for the exception, so it is safe to return the result of the_str.c_str() from what();