0

Writing "perfect" error handling code is known to be hard in C (and very hard in other languages). What is almost always forgotten or discarded by developers is to handle errors when they clean resources. For example ignoring fclose returned value is dangerous.

Anyway, I tried to write a perfect error handling code on a small C89 program that copies in.txt into out.txt. My goal is to write easy to check and maintain code. It should be easy to add a new "resource" or to add a new function call in the middle that could fail. Code duplication must be avoided as much as possible. My basic design is simple: all resources must be initialized to "empty". In case of error I simply jump to "handle_error". At the end I always call "free_resources". The "handle_error" and "free_resources" sections can be executed several times (for example if an error occurs while freeing resources) without issue.

Is my code "perfect"?

#include <stdio.h>

typedef int status;

#define SUCCESS 1
#define FAILURE 0

#define INPUT_FILE "in.txt"
#define OUTPUT_FILE "out.txt"

#define BUFFER_SIZE 2097152

char buffer[BUFFER_SIZE];

status copy_file()
{
        FILE *input_file = NULL;
        FILE *output_file = NULL;
        size_t read_bytes;
        size_t written_bytes;
        status result;

        input_file = fopen(INPUT_FILE, "rb");
        if (!input_file) {
                perror("Failed to open input file");
                goto handle_error;
        }

        output_file = fopen(OUTPUT_FILE, "wb");
        if (!output_file) {
                perror("Failed to open output file");
                goto handle_error;
        }

        while (1) {
                read_bytes = fread(buffer, 1, sizeof(buffer), input_file);
                if (read_bytes != sizeof(buffer) && ferror(input_file)) {
                        fprintf(stderr, "Failed to read from input file.\n");
                        goto handle_error;
                }
                written_bytes = fwrite(buffer, 1, read_bytes, output_file);
                if (written_bytes != read_bytes) {
                        fprintf(stderr, "Failed to write to output file.\n");
                        goto handle_error;
                }
                if (read_bytes != sizeof(buffer))
                        break;
        }

        result = SUCCESS;

free_resources:
        if (output_file) {
                if (fclose(output_file)) {
                        output_file = NULL;
                        perror("Failed to close output file");
                        goto handle_error;
                }
                output_file = NULL;
        }

        if (input_file) {
                if (fclose(input_file)) {
                        input_file = NULL;
                        perror("Failed to close input file");
                        goto handle_error;
                }
                input_file = NULL;
        }

        return result;
handle_error:
        result = FAILURE;
        goto free_resources;
}

int main()
{
        return copy_file() ? 0 : 1;
}
rt15
  • 87
  • 1
  • 1
  • 7
  • 4
    If you have working code and are looking for a review then https://codereview.stackexchange.com/ may be a more appropriate place to ask. My opinion is that code with goto and a flow that causes the same code to execute twice is not perfect. – Retired Ninja Jun 08 '20 at 21:34
  • A better way may be to write wrapper functions, so you don't have to repeat all the error checking. – Barmar Jun 08 '20 at 21:36
  • 3
    Whenever an error message regarding a file operation error that does not include the path used to perform the operation is printed, a kitten dies. IOW, replace "Failed to open input file" with the path used to open the file. – William Pursell Jun 08 '20 at 21:37
  • No. `SUCCESS` and `FAILURE` should probably be `EXIT_SUCCESS` and `EXIT_FAILURE` from ``. You shouldn't need to `goto handle_error` from within the free resources code. You could recognize that you won't have an output file to close if you've not opened the input file. You should recognize that `fread()` might return less than a buffer full (but more than zero) bytes and not be in an error state — especially if the file names are not referring to disk files. You should report the file names in the error messages — that's a particular weakness of `perror()`. – Jonathan Leffler Jun 08 '20 at 21:39
  • See also [Error handling in C code](https://stackoverflow.com/q/385975/15168). – Jonathan Leffler Jun 08 '20 at 21:41
  • 2
    ***`How to write perfect error handling`*** I would like to know it myself .... – 0___________ Jun 08 '20 at 21:52

1 Answers1

0

C language does not provide any direct support for error handling. However, a few methods and variables defined in error.h header file can be used to point out errors using the return statement in a function. In C language, a function returns -1 or NULL value in case of any error and a global variable errno is set with the error code. So the return value can be used to check error while programming.

Whenever a function call is made in C language, a variable named errno is associated with it. It is a global variable, which can be used to identify which type of error was encountered while function execution, based on its value.

C language uses the following functions to represent error messages associated with errno:

  • perror(): returns the string passed to it along with the textual represention of the current errno value.
  • strerror() is defined in string.h library. This method returns a pointer to the string representation of the current errno value.
  • 1
    `a few methods and variables defined in error.h` `error.h` is a GNU extension, it's not part of C language. If you meant `errno.h`, then there are only macros in `errno.h`. – KamilCuk Jun 09 '20 at 10:34