util: add helpers for automatic cleanup on scope exitandroid-s-beta-4android-s-beta-3android-s-beta-4

Leverage the cleanup attribute to attach automatic cleanup calls to
variables.  Now we don't have to worry about FILE* leaks via error
exit paths.

Bug: None
Test: unittests pass

Change-Id: I1a82245c5ef29ef444c1752344ee209202e1456c

5 files changed, 32 insertions, 14 deletions

diff --git a/elfparse.c b/elfparse.c
index e76ebc4..96caa59 100644
--- a/elfparse.c
+++ b/elfparse.c
@@ -11,6 +11,7 @@
 #include <unistd.h>
 
 #include "elfparse.h"
+#include "util.h"
 
 int is_elf_magic (const uint8_t *buf)
 {
@@ -67,7 +68,7 @@ parseElftemplate(32)
 ElfType get_elf_linkage(const char *path)
 {
 	ElfType ret = ELFERROR;
-	FILE *elf_file = NULL;
+	attribute_cleanup_fp FILE *elf_file = NULL;
 	uint8_t pHeader[HEADERSIZE] = "";
 
 	elf_file = fopen(path, "re");
@@ -112,7 +113,6 @@ ElfType get_elf_linkage(const char *path)
 			 */
 			ret = ELFDYNAMIC;
 		}
-		fclose(elf_file);
 	}
 	return ret;
 }
diff --git a/libminijail.c b/libminijail.c
index 55ff4af..89a4375 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -1135,7 +1135,7 @@ void API minijail_parse_seccomp_filters(struct minijail *j, const char *path)
 	if (!seccomp_should_use_filters(j))
 		return;
 
-	FILE *file = fopen(path, "re");
+	attribute_cleanup_fp FILE *file = fopen(path, "re");
 	if (!file) {
 		pdie("failed to open seccomp filter file '%s'", path);
 	}
@@ -1144,13 +1144,12 @@ void API minijail_parse_seccomp_filters(struct minijail *j, const char *path)
 		die("failed to compile seccomp filter BPF program in '%s'",
 		    path);
 	}
-	fclose(file);
 }
 
 void API minijail_parse_seccomp_filters_from_fd(struct minijail *j, int fd)
 {
 	char *fd_path, *path;
-	FILE *file;
+	attribute_cleanup_fp FILE *file = NULL;
 
 	if (!seccomp_should_use_filters(j))
 		return;
@@ -1172,7 +1171,6 @@ void API minijail_parse_seccomp_filters_from_fd(struct minijail *j, int fd)
 		    fd);
 	}
 	free(path);
-	fclose(file);
 }
 
 void API minijail_set_seccomp_filters(struct minijail *j,
diff --git a/minijail0_cli.c b/minijail0_cli.c
index 3461579..508ead9 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -488,20 +488,18 @@ static void set_remount_mode(struct minijail *j, const char *mode)
 static void read_seccomp_filter(const char *filter_path,
 				struct sock_fprog *filter)
 {
-	FILE *f = fopen(filter_path, "re");
+	attribute_cleanup_fp FILE *f = fopen(filter_path, "re");
 	if (!f) {
 		fprintf(stderr, "failed to open %s: %m", filter_path);
 		exit(1);
 	}
 	off_t filter_size = 0;
 	if (fseeko(f, 0, SEEK_END) == -1 || (filter_size = ftello(f)) == -1) {
-		fclose(f);
 		fprintf(stderr, "failed to get file size of %s: %m",
 			filter_path);
 		exit(1);
 	}
 	if (filter_size % sizeof(struct sock_filter) != 0) {
-		fclose(f);
 		fprintf(stderr,
 			"filter size (%" PRId64
 			") of %s is not a multiple of %zu: %m",
@@ -514,11 +512,9 @@ static void read_seccomp_filter(const char *filter_path,
 	filter->filter = xmalloc(filter_size);
 	if (fread(filter->filter, sizeof(struct sock_filter), filter->len, f) !=
 	    filter->len) {
-		fclose(f);
 		fprintf(stderr, "failed read %s: %m", filter_path);
 		exit(1);
 	}
-	fclose(f);
 }
 
 static void usage(const char *progn)
diff --git a/syscall_filter.c b/syscall_filter.c
index fcdbaa8..945e6be 100644
--- a/syscall_filter.c
+++ b/syscall_filter.c
@@ -670,7 +670,8 @@ int compile_file(const char *filename, FILE *policy_file,
 				goto free_line;
 			}
 
-			FILE *included_file = fopen(filename, "re");
+			attribute_cleanup_fp FILE *included_file =
+			    fopen(filename, "re");
 			if (included_file == NULL) {
 				compiler_pwarn(&state, "fopen('%s') failed",
 					       filename);
@@ -683,11 +684,9 @@ int compile_file(const char *filename, FILE *policy_file,
 					 include_level + 1) == -1) {
 				compiler_warn(&state, "'@include %s' failed",
 					      filename);
-				fclose(included_file);
 				ret = -1;
 				goto free_line;
 			}
-			fclose(included_file);
 			continue;
 		}
 
diff --git a/util.h b/util.h
index 6c5ed9d..b2815fb 100644
--- a/util.h
+++ b/util.h
@@ -55,6 +55,31 @@ extern "C" {
 #define attribute_printf(format_idx, check_idx) \
 	__attribute__((__format__(__printf__, format_idx, check_idx)))
 
+/*
+ * Mark a local variable for automatic cleanup when exiting its scope.
+ * See attribute_cleanup_fp as an example below.
+ * Make sure any variable using this is always initialized to something.
+ * @func The function to call on (a pointer to) the variable.
+ */
+#define attribute_cleanup(func) \
+	__attribute__((__cleanup__(func)))
+
+/*
+ * Automatically close a FILE* when exiting its scope.
+ * Make sure the pointer is always initialized.
+ * Some examples:
+ *   attribute_cleanup_fp FILE *fp = fopen(...);
+ *   attribute_cleanup_fp FILE *fp = NULL;
+ *   ...
+ *   fp = fopen(...);
+ */
+#define attribute_cleanup_fp attribute_cleanup(_cleanup_fp)
+static inline void _cleanup_fp(FILE **fp)
+{
+	if (*fp)
+		fclose(*fp);
+}
+
 /* clang-format off */
 #define die(_msg, ...) \
 	do_fatal_log(LOG_ERR, "libminijail[%d]: " _msg, getpid(), ## __VA_ARGS__)