Skip to content

Instantly share code, notes, and snippets.

@oconnor663
Last active February 27, 2023 03:27
Show Gist options
  • Save oconnor663/b0b32f25301ed5a1af4d07053edf7036 to your computer and use it in GitHub Desktop.
Save oconnor663/b0b32f25301ed5a1af4d07053edf7036 to your computer and use it in GitHub Desktop.
inner/outer RAII in C
#include <errno.h>
#include <stdio.h>
// We want a function that creates a couple files. Here's the naive approach:
int hardcoded_cleanup(const char *path1, const char *path2) {
FILE *file1 = fopen(path1, "w");
if (file1 == NULL) {
return errno;
}
FILE *file2 = fopen(path2, "w");
if (file2 == NULL) {
int ret = errno;
fclose(file1);
return ret;
}
int n = fwrite("foo", 1, 3, file1);
if (n != 3) {
int ret = errno;
fclose(file1);
fclose(file2);
return ret;
}
n = fwrite("bar", 1, 3, file2);
if (n != 3) {
int ret = errno;
fclose(file1);
fclose(file2);
return ret;
}
if (fclose(file1) == EOF) {
int ret = errno;
fclose(file2);
return ret;
}
if (fclose(file2) == EOF) {
return errno;
}
return 0;
}
// This sucks. We're copying lots of cleanup code, and the cleanup code is
// subtly different in different places, which makes it vulnerable to copy-paste
// mistakes. A common solution to this is to unify the error handling at the
// bottom and use gotos:
int goto_cleanup(const char *path1, const char *path2) {
int ret = 0;
FILE *file1 = NULL;
FILE *file2 = NULL;
file1 = fopen(path1, "w");
if (file1 == NULL) {
ret = errno;
goto cleanup;
}
file2 = fopen(path2, "w");
if (file2 == NULL) {
ret = errno;
goto cleanup;
}
int n = fwrite("foo", 1, 3, file1);
if (n != 3) {
ret = errno;
goto cleanup;
}
n = fwrite("bar", 1, 3, file2);
if (n != 3) {
ret = errno;
goto cleanup;
}
int close_ret = fclose(file1);
file1 = NULL;
if (close_ret == EOF) {
ret = errno;
goto cleanup;
}
close_ret = fclose(file2);
file2 = NULL;
if (close_ret == EOF) {
ret = errno;
goto cleanup;
}
cleanup:
if (file1 != NULL) {
fclose(file1);
}
if (file2 != NULL) {
fclose(file2);
}
return ret;
}
// This is somewhat better. The error handling is more consistent, and we're
// less likely to make mistakes. But it's got a few downsides:
//
// - Goto is problematic, and we'd rather ban it.
// - If we forget to NULL-initialize all our variables at the top, the cleanup
// block will operate on uninitialized data, probably in an error case that's
// hard to test. There's no compiler warning for that.
// - Going to cleanup without assigning to `ret` is almost certainly a bug, but
// there's no compiler warning for that either. This is easier to catch when
// every error handling branch looks the same, as in this example, but that's
// not always the case.
//
// Here's an alternative inner/outer resource management pattern that I prefer:
int _inner_function(const char *path1, const char *path2, FILE **file1, FILE **file2) {
// This function is private. It doesn't do any cleanup.
*file1 = fopen(path1, "w");
if (*file1 == NULL) {
return errno;
}
*file2 = fopen(path2, "w");
if (*file2 == NULL) {
return errno;
}
int n = fwrite("foo", 1, 3, *file1);
if (n != 3) {
return errno;
}
n = fwrite("bar", 1, 3, *file2);
if (n != 3) {
return errno;
}
int close_ret = fclose(*file1);
*file1 = NULL;
if (close_ret == EOF) {
return errno;
}
close_ret = fclose(*file2);
*file2 = NULL;
if (close_ret == EOF) {
return errno;
}
return 0;
}
int outer_function(const char *path1, const char *path2) {
// This function is public. It has no early returns.
FILE *file1 = NULL;
FILE *file2 = NULL;
int ret = _inner_function(path1, path2, &file1, &file2);
if (file1 != NULL) {
fclose(file1);
}
if (file2 != NULL) {
fclose(file2);
}
return ret;
}
// The inner/outer approach above has several benefits:
//
// - No gotos.
// - Resource declarations are right next to cleanup code, so it's easier to
// see that all resources are NULL-initialized and that we can't call any
// free/close functions on an uninitialized pointer.
// - The compiler type-checks all of our error returns. No forgotten error
// codes.
// - The "interesting" code in the inner function is tighter, with fewer
// copy-pasted lines.
//
// On the other hand, it has a couple downsides:
//
// - Writing two functions instead of one is annoying.
// - It's easy to forget to dereference a pointer in one of the NULL checks, or
// with functions like memcpy that take void*. The compiler doesn't catch
// these mistakes, and the result can be a silent error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment