This article looks at handling errors in C APIs using an if-error-else coding pattern. It describes the issues and options when using this coding style, with the full example of the copy file example rewritten.

Introduction

If opening a file fails (e.g. because of the file permissions) fopen returns a null pointer. The code has to test for this case. One way is to use if-else blocks like below:

1
2
3
4
5
6
7
8
9
10
11
12
13

FILE * f = fopen("file.name", "rb");
if (f)
{
  // more
  // code
  // here

  fclose(f);
}
else
{
  perror("Failed to open file.name");
}

One problem is that, as more code gets added, both releasing resources (fclose) and logging the error (perror) move away from the call they relate to (fopen). That can be partially addressed as:

1
2
3
4
5
6
7
8
9
10
11
12
13

FILE * f = fopen("file.name", "rb");
if ( ! f)
{
  perror("Failed to open file.name");
}
else
{
  // more
  // code
  // here

  fclose(f);
}

In this case it’s just fclose that moves away from the related function (fopen) as more code gets added. I refer to this coding style the if-error-else coding pattern.

For functions that need not release any resource, like fwrite, the error handing can stay close to the function it relates to:

1
2
3
4
5
6
7
8
9
10
11

size_t write_count = fwrite(buffer , 1, read_count, dst);
if (write_count != read_count)
{
  perror("Failed to write to destination file");
}
else
{
  // more
  // code
  // here
}

Issues

The first issue with this approach is the repetition. It’s a major issue. Every time we call fopen using this coding pattern we also need to repeat:

  • the if condition
  • the error handling
  • the else keyword
  • the fclose call
  • and 4 curly brackets

For the copy file example this approach takes the code from 28 lines of code to 68 lines of code, more than double. So much repetition obscures application core logic and has cascading effects that results on errors when coding and time waste when reading code.

On one side this coding style at least tries to check systematically for errors. But, because of the repetition, there is repeated scope for error that for any but non-trivial projects results in mistakes that cumulate to a muddy code quality.

One apparently trivial issue is the case of irregular error checking. Notice how in the example below fread does not have a else branch, instead it breaks out of the for loop on error. Also the success path is buried somewhere in the middle of the code. This kind of things have no impact on execution they just slow down understanding a program and the ability to change it easily.

Repetition leads to the chance of incorrect error test messages. The code to open the file was copy-pasted and the same message "Failed to open source file" might also be logged when failing to open the destination file.

Repetition also leads to the chance of missing or incorrect error handling.

The code repetition can be addressed using a C++ RAII approach.

The second issue is the ever growing nesting depth. For the simple example below we get to 6 levels deep compared with 2 levels in the example with no error handling.

Full code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68

#include <stdio.h>
#include <stdlib.h>

int main ()
{
  int return_value = 1;

  FILE * src = fopen("src.bin", "rb");
  if (0 == src)
  {
    perror("Failed to open source file");
  }
  else
  {
    FILE * dst = fopen("dst.bin", "wb");
    if (0 == dst)
    {
      perror("Failed to open destination file");
    }
    else
    {
      const size_t buffer_size = 1024;
      char * buffer = malloc(buffer_size);
      if (0 == buffer)
      {
        fputs("Failed to allocate buffer\n", stderr);
      }
      else
      {
        for(;;)
        {
          size_t read_count = fread(buffer, 1, buffer_size, src);
          if ((read_count != buffer_size) && ferror(src))
          {
            perror("Failed to read from source file");
            break;
          }

          if (read_count > 0)
          {
            size_t write_count = fwrite(buffer, 1, read_count, dst);
            if (write_count != read_count)
            {
              perror("Failed to write to destination file");
              break;
            }
            fputs(".", stdout);
          }

          if (feof(src))
          {
            return_value = 0;
            fputs("\nSUCCESS\n", stdout);
            break;
          }
        }

        free(buffer);
      }

      fclose(dst);
    }

    fclose(src);
  }

  return return_value;
}

Summary

You need to understand how to handle errors. Logically the code above does the right thing, in that it is what we want the computer to execute. However as a coding style the if-error-else approach is repetitive and error prone. Don’t use it.