The Good, the Bad and the Ugly, Part 2

Why was the code from Part 1 bad ? For example, it did not check the result of a number of function invocations. Although in the strictest sense there is nothing actually wrong with that, as it is envisioned by the C language, more often than not all it tells the reader is that the writer just didn't think of it. The language also provides the way to tell the maintenance programmer "I am really not interested in the result, only in the side effects" (if you're not interested in the result of a function without side effects, there is no point in calling it, does it ?): cast the result to void, e.g.

/*
 popen-test: experiment with popen(), Part 2.

 Copyright © 2006  Davide Bolcioni <dbolcion@libero.it>
                                                                              
 Distributed under the GPL,
 see http://www.gnu.org/copyleft/gpl.html.

*/

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>

int main(int argc, char* argv[])
{
 int rc = EXIT_FAILURE; /* Optimist */

 if (argc > 1) {
   FILE* sink;

   (void)fflush(NULL); /* all open files */
   sink = popen(argv[1], "w");

   if (sink) {
     int status;
     int i;

     /* NB: deliberately allow to test for no output sent */

     for (i = 2; i < argc; i++) {
       (void)fputs(argv[i], sink);
     }

     status = pclose(sink);
    
     /* The return value is funny */

     if (status == -1) {
       (void)fprintf(stderr,
               "%s: pclose() failed.\n",
               argv[0]);
     }
     else if (WIFEXITED(status)) {
       rc = WEXITSTATUS(status);
       (void)fprintf(stderr,
               "%s: child did an exit(%d).\n",
               argv[0], rc);
     }
     else if (WIFSIGNALED(status)) {
       (void)fprintf(stderr,
               "%s: child terminated by signal %d.\n",
               argv[0], WTERMSIG(status));
     }
     else if (WIFSTOPPED(status)) {
       (void)fprintf(stderr,
               "%s: child stopped with signal %d.\n",
               argv[0], WSTOPSIG(status));
     }
     else {
       (void)fprintf(stderr,
               "%s: I don't think we're in Kansas anymore, Toto.\n",
               argv[0]);
     }
   }
   else {
     (void)fprintf(stderr,
             "%s: popen(\"%s\") failed.\n",
             argv[0], argv[1]);
   }
 }
 else {
   (void)fprintf(stderr,
           "Usage: %s <command> [<arg>]...\n", argv[0]);
 }

 return rc;
}

/*
 vim:sw=2:nowrap
*/

As you can see, the code is definitely getting uglier. The above tells us that the writer was not interested in the result of the functions, but tells us very little about why: he might be ignoring a result he should heed.

Handling the result of a function which is called primarily for its side effects is harder than it seems, because the result typically tells if the function was "successful", i.e. the intended side effects actually occurred. If the function returns failure, the caller must handle the fact; as we'll see in the next part, this significantly complicates the code.

Because of the effort involved in handling failures, there is a strong temptation to pretend that side effect causing functions cannot fail; some of you might even say that in the above concrete case, the fprintf() invocations cannot fail. To let the maintenance programmer know that a specific invocation cannot fail, just use assert():

   assert(fprintf(stderr,
          "Usage: %s <command> [<arg>]...\n", argv[0]) > 0);

This code is actually wrong; if the code is built with NDEBUG, the assert() gets removed during preprocessing and no fprintf() invocation would occur in its place. The ugly-as-hell but correct option is to remember the result in a variable:

  int written = fprintf(stderr,
                "Usage: %s <command> [<arg>]...\n", argv[0]);
  assert(written > 0);

which, unless we want to get into reusing variables or to devise a different variable name for each invocation of fprintf(), gets uglier still:

  {
    int written = fprintf(stderr,
                  "Usage: %s <command> [<arg>]...\n", argv[0]);
    assert(written > 0);
  }

The assert() condition in the examples above is not very tight, since fprintf returns the number of characters written; we're essentially saying that we're positive that fprintf() wrote something. If we wanted to say that we're sure that fprintf() unfailingly wrote everything, we'd have to write a much more involved condition.

There is a trap hidden in there, as in the example below

  {
    int written = fprintf(stderr,
                  "%s", argv[1]);
    assert(written > 0);
  }

the assertion would fire incorrectly if argv[1] had zero length, and the maintenance programmer would have to decide whether the original author did not look up what the return value of fprintf() means, or argv[1] was supposed to have a positive length.

So, when handling results for functions called for their side effect, we can state our convincement that a failure cannot occur with assert(), admit our lack of interest in the matter by casting to void, or handle the failure ourselves, which we'll discuss in Part 3.