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;
}
SUCCESSandFAILUREshould probably beEXIT_SUCCESSandEXIT_FAILUREfrom<stdlib.h>. You shouldn't need togoto handle_errorfrom 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 thatfread()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 ofperror().How to write perfect error handlingI would like to know it myself ....