[apparmor] [PATCH v2 07/42] Use the gcc cleanup extension attribute to handle closing temp files

Tyler Hicks tyhicks at canonical.com
Fri Mar 6 21:48:23 UTC 2015


From: John Johansen <john.johansen at canonical.com>

While some of these allocations will go away as we convert to C++,
some of these need to stay C as the are going to be moved into a
library to support loading cache from init daemons etc.

For the bits that will eventually be C++ this helps clean things up,
in the interim.

TODO: apply to libapparmor as well

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 parser/features.c         |  9 +++------
 parser/lib.c              | 17 +++++++++++++++++
 parser/lib.h              |  4 ++++
 parser/network.c          |  5 +++--
 parser/parser_include.c   |  5 +++--
 parser/parser_interface.c | 12 +++---------
 parser/parser_main.c      | 15 ++++-----------
 parser/policy_cache.c     |  8 ++------
 8 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/parser/features.c b/parser/features.c
index d183365..673fe4f 100644
--- a/parser/features.c
+++ b/parser/features.c
@@ -72,7 +72,8 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
 	fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "%s {", name);
 
 	if (S_ISREG(st->st_mode)) {
-		int len, file;
+		autoclose int file = -1;
+		int len;
 		int remaining = fst->size - (fst->pos - *fst->buffer);
 
 		file = openat(dirfd(dir), name, O_RDONLY);
@@ -98,7 +99,6 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
 			PDEBUG("Error reading feature file '%s'\n", name);
 			return -1;
 		}
-		close(file);
 	} else if (S_ISDIR(st->st_mode)) {
 		if (dirat_for_each(dir, name, fst, features_dir_cb))
 			return -1;
@@ -124,7 +124,7 @@ static char *handle_features_dir(const char *filename, char **buffer, int size,
 
 char *load_features_file(const char *name) {
 	char *buffer;
-	FILE *f = NULL;
+	autofclose FILE *f = NULL;
 	size_t size;
 
 	f = fopen(name, "r");
@@ -140,14 +140,11 @@ char *load_features_file(const char *name) {
 		goto fail;
 	buffer[size] = 0;
 
-	fclose(f);
 	return buffer;
 
 fail:
 	int save = errno;
 	free(buffer);
-	if (f)
-		fclose(f);
 	errno = save;
 	return NULL;
 }
diff --git a/parser/lib.c b/parser/lib.c
index fb9df49..5340971 100644
--- a/parser/lib.c
+++ b/parser/lib.c
@@ -40,6 +40,23 @@ void __autofree(void *p)
 	free(*_p);
 }
 
+void __autoclose(int *fd)
+{
+	if (*fd != -1) {
+		/* if close was interrupted retry */
+		while(close(*fd) == -1 && errno == EINTR);
+		*fd = -1;
+	}
+}
+
+void __autofclose(FILE **f)
+{
+	if (*f) {
+		fclose(*f);
+		*f = NULL;
+	}
+}
+
 /**
  * dirat_for_each: iterate over a directory calling cb for each entry
  * @dir: already opened directory (MAY BE NULL)
diff --git a/parser/lib.h b/parser/lib.h
index 73e1ffc..f047fd8 100644
--- a/parser/lib.h
+++ b/parser/lib.h
@@ -4,7 +4,11 @@
 #include <dirent.h>
 
 #define autofree __attribute((cleanup(__autofree)))
+#define autoclose __attribute((cleanup(__autoclose)))
+#define autofclose __attribute((cleanup(__autofclose)))
 void __autofree(void *p);
+void __autoclose(int *fd);
+void __autofclose(FILE **f);
 
 int dirat_for_each(DIR *dir, const char *name, void *data,
 		   int (* cb)(DIR *, const char *, struct stat *, void *));
diff --git a/parser/network.c b/parser/network.c
index 80cce67..f26378b 100644
--- a/parser/network.c
+++ b/parser/network.c
@@ -25,6 +25,7 @@
 #include <sstream>
 #include <map>
 
+#include "lib.h"
 #include "parser.h"
 #include "profile.h"
 #include "parser_yacc.h"
@@ -154,7 +155,8 @@ static struct network_tuple network_mappings[] = {
 static size_t kernel_af_max(void) {
 	char buffer[32];
 	int major;
-	int fd, res;
+	autoclose int fd = -1;
+	int res;
 
 	if (!net_af_max_override) {
 		return 0;
@@ -168,7 +170,6 @@ static size_t kernel_af_max(void) {
 		/* fall back to default provided during build */
 		return 0;
 	res = read(fd, &buffer, sizeof(buffer) - 1);
-	close(fd);
 	if (res <= 0)
 		return 0;
 	buffer[res] = '\0';
diff --git a/parser/parser_include.c b/parser/parser_include.c
index 234d2d2..f4f1e23 100644
--- a/parser/parser_include.c
+++ b/parser/parser_include.c
@@ -45,6 +45,8 @@
 #include <unistd.h>
 #include <errno.h>
 #include <dirent.h>
+
+#include "lib.h"
 #include "parser.h"
 #include "parser_include.h"
 
@@ -176,7 +178,7 @@ int add_search_dir(const char *dir)
    SUBDOMAIN_PATH=/etc/subdomain.d/include */
 void parse_default_paths(void)
 {
-	FILE *f;
+	autofclose FILE *f;
 	char buf[1024];
 	char *t, *s;
 	int saved_npath = npath;
@@ -202,7 +204,6 @@ void parse_default_paths(void)
 			} while (s != NULL);
 		}
 	}
-	fclose(f);
 
 	/* if subdomain.conf doesn't set a base search dir set it to this */
 out:
diff --git a/parser/parser_interface.c b/parser/parser_interface.c
index 485c30b..7829b22 100644
--- a/parser/parser_interface.c
+++ b/parser/parser_interface.c
@@ -479,7 +479,7 @@ void sd_serialize_top_profile(std::ostringstream &buf, Profile *profile)
 int cache_fd = -1;
 int __sd_serialize_profile(int option, Profile *prof)
 {
-	int fd = -1;
+	autoclose int fd = -1;
 	int error = -ENOMEM, size, wsize;
 	std::ostringstream work_area;
 	autofree char *filename = NULL;
@@ -594,9 +594,6 @@ int __sd_serialize_profile(int option, Profile *prof)
 		}
 	}
 
-	if (fd != -1)
-		close(fd);
-
 	if (!prof->hat_table.empty() && option != OPTION_REMOVE) {
 		if (load_flattened_hats(prof, option) == 0)
 			return 0;
@@ -641,7 +638,7 @@ static int write_buffer(int fd, char *buffer, int size, bool set)
 
 int sd_load_buffer(int option, char *buffer, int size)
 {
-	int fd = -1;
+	autoclose int fd = -1;
 	int error, bsize;
 	autofree char *filename = NULL;
 
@@ -666,8 +663,7 @@ int sd_load_buffer(int option, char *buffer, int size)
 	if (fd < 0) {
 		PERROR(_("Unable to open %s - %s\n"), filename,
 		       strerror(errno));
-		error = -errno;
-		goto out;
+		return -errno;
 	}
 
 	if (kernel_supports_setload) {
@@ -688,8 +684,6 @@ int sd_load_buffer(int option, char *buffer, int size)
 				break;
 		}
 	}
-	close(fd);
 
-out:
 	return error;
 }
diff --git a/parser/parser_main.c b/parser/parser_main.c
index f18d5f4..342ee14 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -498,7 +498,7 @@ static int process_args(int argc, char *argv[])
 static int process_config_file(const char *name)
 {
 	char *optarg;
-	FILE *f;
+	autofclose FILE *f = NULL;
 	int c, o;
 
 	f = fopen(name, "r");
@@ -507,7 +507,6 @@ static int process_config_file(const char *name)
 
 	while ((c = getopt_long_file(f, long_options, &optarg, &o)) != -1)
 		process_arg(c, optarg);
-	fclose(f);
 	return 1;
 }
 
@@ -553,7 +552,7 @@ int have_enough_privilege(void)
 
 static void set_features_by_match_file(void)
 {
-	FILE *ms = fopen(MATCH_FILE, "r");
+	autofclose FILE *ms = fopen(MATCH_FILE, "r");
 	if (ms) {
 		autofree char *match_string = (char *) malloc(1000);
 		if (!match_string)
@@ -563,14 +562,10 @@ static void set_features_by_match_file(void)
 		if (strstr(match_string, " perms=c"))
 			perms_create = 1;
 		kernel_supports_network = 1;
-		goto out;
+		return;
 	}
 no_match:
 	perms_create = 1;
-
-out:
-	if (ms)
-		fclose(ms);
 }
 
 static void set_supported_features(void) {
@@ -618,7 +613,7 @@ int process_binary(int option, const char *profilename)
 	autofree char *buffer = NULL;
 	int retval = 0, size = 0, asize = 0, rsize;
 	int chunksize = 1 << 14;
-	int fd;
+	autoclose int fd = -1;
 
 	if (profilename) {
 		fd = open(profilename, O_RDONLY);
@@ -648,8 +643,6 @@ int process_binary(int option, const char *profilename)
 			size += rsize;
 	} while (rsize > 0);
 
-	close(fd);
-
 	if (rsize == 0)
 		retval = sd_load_buffer(option, buffer, size);
 	else
diff --git a/parser/policy_cache.c b/parser/policy_cache.c
index f2909b9..b6f8299 100644
--- a/parser/policy_cache.c
+++ b/parser/policy_cache.c
@@ -40,13 +40,12 @@ const char header_string[] = "\004\010\000version\000\002";
 bool valid_cached_file_version(const char *cachename)
 {
 	char buffer[16];
-	FILE *f;
+	autofclose FILE *f;
 	if (!(f = fopen(cachename, "r"))) {
 		PERROR(_("Error: Could not read cache file '%s', skipping...\n"), cachename);
 		return false;
 	}
 	size_t res = fread(buffer, 1, 16, f);
-	fclose(f);
 	if (res < 16) {
 		if (debug_cache)
 			pwarn("%s: cache file '%s' invalid size\n", progname, cachename);
@@ -111,7 +110,7 @@ int clear_cache_files(const char *path)
 int create_cache(const char *cachedir, const char *path, const char *features)
 {
 	struct stat stat_file;
-	FILE * f = NULL;
+	autofclose FILE * f = NULL;
 
 	if (clear_cache_files(cachedir) != 0)
 		goto error;
@@ -122,9 +121,6 @@ create_file:
 		if (fwrite(features, strlen(features), 1, f) != 1 )
 			goto error;
 
-		fclose(f);
-
-
 		return 0;
 	}
 
-- 
2.1.4




More information about the AppArmor mailing list