* * * * *
I told you handing errors was error prone
> From: Mark Grosberg <XXXXXXXXXXXXXXXXX>
> To:
[email protected]
> Subject: Blog post.
> Date: Wed, 2 Dec 2009 15:59:55 -0500 (EST)
>
> I find it even more amusing that you didn't get the error handling right in
> the create_socket() on your current blog post.
>
> Notice that you leak the socket and/or memory in the error cases. I guess
> it really is hard to handle errors. ;-)
>
> Sorry, I just had to take this cheap shot!
>
> -MYG
>
Heh.
Yup, I blew it again for demonstration purposes.
The code I posted yesterday [1] was actually pulled from a current project
where the create_socket() is only called during initialization and if it
fails, the program exits. Since I'm on a Unix system, the “lost” resources
like memory and sockets are automatically reclaimed. Not all operating
systems are nice like this.
There are a few ways to fix this. One, is to use a langauge that handles such
details automatically with garbage collection [2], but I'm using C so that's
not an option. The second one is to add cleanup code at each point we exit,
but using that we end up with code that looks like:
> /* ... */
>
> if fcntl(listen->sock,F_GETFL,0) == -1)
> {
> perror("fcntl(GETFL)");
> close(listen->socket);
> free(listen);
> return -1;
> }
>
> if (fcntl(listen->sock,F_SETFL,rc | O_NONBLOCK) == -1)
> {
> perror("fcntl(SETFL)");
> close(listen->socket);
> free(listen);
> return -1;
> }
>
> if (bind(listen->sock,paddr,saddr) == -1)
> {
> perror("bind()");
> close(listen->socket);
> free(listen);
> return -1;
> }
>
> /* ... */
>
Lots of duplicated code and the more complex the routine, the more complex
the cleanup and potential to leak memory (or other resources like files and
network connections). The other option looks like:
> /* ... */
>
> if fcntl(listen->sock,F_GETFL,0) == -1)
> {
> perror("fcntl(GETFL)");
> goto create_socket_cleanup;
> }
>
> if (fcntl(listen->sock,F_SETFL,rc | O_NONBLOCK) == -1)
> {
> perror("fcntl(SETFL)");
> goto create_socket_cleanup;
> }
>
> if (bind(listen->sock,paddr,saddr) == -1)
> {
> perror("bind()");
> goto create_socket_cleanup;
> }
>
> /* rest of code */
>
> return listen->sock; /* everything is okay */
>
> create_socket_cleanup:
> close(listen->sock);
> create_socket_cleanup_mem:
> free(listen);
> return -1;
> }
>
This uses the dreaded goto construct, but is one of the few places that it's
deemed “okay” to use goto, for cleaning up errors. No code duplication, but
you need to make sure you cleanup (or unwind, or whatever) in reverse order.
So yeah, error handling … maddening.
I still wish there was a better way …
[1]
gopher://gopher.conman.org/0Phlog:2009/12/01.2
[2]
http://en.wikipedia.org/wiki/Garbage_collection_(computer_science)
Email author at
[email protected]