[apparmor] [PATCH v2 06/42] Use the gcc cleanup extension attribute to handle freeing temp allocations

Tyler Hicks tyhicks at canonical.com
Fri Mar 6 21:48:22 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/lib.c              | 11 ++++++++---
 parser/lib.h              |  3 +++
 parser/parser_interface.c | 13 ++++---------
 parser/parser_lex.l       | 16 ++++------------
 parser/parser_main.c      | 39 ++++++++++++---------------------------
 5 files changed, 31 insertions(+), 51 deletions(-)

diff --git a/parser/lib.c b/parser/lib.c
index 85e39e0..fb9df49 100644
--- a/parser/lib.c
+++ b/parser/lib.c
@@ -33,6 +33,13 @@
 #include "lib.h"
 #include "parser.h"
 
+/* automaticly free allocated variables tagged with autofree on fn exit */
+void __autofree(void *p)
+{
+	void **_p = (void**)p;
+	free(*_p);
+}
+
 /**
  * dirat_for_each: iterate over a directory calling cb for each entry
  * @dir: already opened directory (MAY BE NULL)
@@ -62,7 +69,7 @@
 int dirat_for_each(DIR *dir, const char *name, void *data,
 		   int (* cb)(DIR *, const char *, struct stat *, void *))
 {
-	struct dirent *dirent = NULL;
+	autofree struct dirent *dirent = NULL;
 	DIR *d = NULL;
 	int error;
 
@@ -132,7 +139,6 @@ int dirat_for_each(DIR *dir, const char *name, void *data,
 
 	if (d != dir)
 		closedir(d);
-	free(dirent);
 
 	return 0;
 
@@ -140,7 +146,6 @@ fail:
 	error = errno;
 	if (d && d != dir)
 		closedir(d);
-	free(dirent);
 	errno = error;
 
 	return -1;
diff --git a/parser/lib.h b/parser/lib.h
index aac2223..73e1ffc 100644
--- a/parser/lib.h
+++ b/parser/lib.h
@@ -3,6 +3,9 @@
 
 #include <dirent.h>
 
+#define autofree __attribute((cleanup(__autofree)))
+void __autofree(void *p);
+
 int dirat_for_each(DIR *dir, const char *name, void *data,
 		   int (* cb)(DIR *, const char *, struct stat *, void *));
 
diff --git a/parser/parser_interface.c b/parser/parser_interface.c
index 0d6626d..485c30b 100644
--- a/parser/parser_interface.c
+++ b/parser/parser_interface.c
@@ -28,6 +28,7 @@
 #include <string>
 #include <sstream>
 
+#include "lib.h"
 #include "parser.h"
 #include "profile.h"
 #include "libapparmor_re/apparmor_re.h"
@@ -374,13 +375,11 @@ void sd_serialize_profile(std::ostringstream &buf, Profile *profile,
 	sd_write_struct(buf, "profile");
 	if (flattened) {
 		assert(profile->parent);
-		char *name = (char *) malloc(3 + strlen(profile->name) +
-				    strlen(profile->parent->name));
+		autofree char *name = (char *) malloc(3 + strlen(profile->name) + strlen(profile->parent->name));
 		if (!name)
 			return;
 		sprintf(name, "%s//%s", profile->parent->name, profile->name);
 		sd_write_string(buf, name, NULL);
-		free(name);
 	} else {
 		sd_write_string(buf, profile->name, NULL);
 	}
@@ -483,7 +482,7 @@ int __sd_serialize_profile(int option, Profile *prof)
 	int fd = -1;
 	int error = -ENOMEM, size, wsize;
 	std::ostringstream work_area;
-	char *filename = NULL;
+	autofree char *filename = NULL;
 
 	switch (option) {
 	case OPTION_ADD:
@@ -523,8 +522,6 @@ int __sd_serialize_profile(int option, Profile *prof)
 
 	error = 0;
 
-	free(filename);
-
 	if (option == OPTION_REMOVE) {
 		char *name, *ns = NULL;
 		int len = 0;
@@ -646,7 +643,7 @@ int sd_load_buffer(int option, char *buffer, int size)
 {
 	int fd = -1;
 	int error, bsize;
-	char *filename = NULL;
+	autofree char *filename = NULL;
 
 	/* TODO: push backup into caller */
 	if (!kernel_load)
@@ -694,7 +691,5 @@ int sd_load_buffer(int option, char *buffer, int size)
 	close(fd);
 
 out:
-	free(filename);
-
 	return error;
 }
diff --git a/parser/parser_lex.l b/parser/parser_lex.l
index 2f6f914..0456843 100644
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -125,15 +125,13 @@ static int include_dir_cb(DIR *dir unused, const char *name, struct stat *st,
 {
 	struct cb_struct *d = (struct cb_struct *) data;
 
-	char *path;
+	autofree char *path = NULL;
 
 	if (asprintf(&path, "%s/%s", d->fullpath, name) < 0)
 		yyerror("Out of memory");
 
-	if (is_blacklisted(name, path)) {
-		free(path);
+	if (is_blacklisted(name, path))
 		return 0;
-	}
 
 	if (S_ISREG(st->st_mode)) {
 		if (!(yyin = fopen(path,"r")))
@@ -144,8 +142,6 @@ static int include_dir_cb(DIR *dir unused, const char *name, struct stat *st,
 		yypush_buffer_state(yy_create_buffer(yyin, YY_BUF_SIZE));
 	}
 
-	free(path);
-
 	return 0;
 }
 
@@ -153,7 +149,7 @@ void include_filename(char *filename, int search)
 {
 	FILE *include_file = NULL;
 	struct stat my_stat;
-	char *fullpath = NULL;
+	autofree char *fullpath = NULL;
 
 	if (search) {
 		if (preprocess_only)
@@ -188,9 +184,6 @@ void include_filename(char *filename, int search)
 				  " '%s' in '%s'"), fullpath, filename);;
 		}
 	}
-
-	if (fullpath)
-		free(fullpath);
 }
 
 %}
@@ -281,9 +274,8 @@ LT_EQUAL	<=
 
 <INCLUDE>{
 	(\<([^\> \t\n]+)\>|\"([^\" \t\n]+)\")	{	/* <filename> */
-		char *filename = strndup(yytext, yyleng - 1);
+		autofree char *filename = strndup(yytext, yyleng - 1);
 		include_filename(filename + 1, *filename == '<');
-		free(filename);
 		POP_NODUMP();
 	}
 
diff --git a/parser/parser_main.c b/parser/parser_main.c
index f075618..f18d5f4 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -555,16 +555,13 @@ static void set_features_by_match_file(void)
 {
 	FILE *ms = fopen(MATCH_FILE, "r");
 	if (ms) {
-		char *match_string = (char *) malloc(1000);
+		autofree char *match_string = (char *) malloc(1000);
 		if (!match_string)
 			goto no_match;
-		if (!fgets(match_string, 1000, ms)) {
-			free(match_string);
+		if (!fgets(match_string, 1000, ms))
 			goto no_match;
-		}
 		if (strstr(match_string, " perms=c"))
 			perms_create = 1;
-		free(match_string);
 		kernel_supports_network = 1;
 		goto out;
 	}
@@ -618,7 +615,7 @@ static void set_supported_features(void) {
 
 int process_binary(int option, const char *profilename)
 {
-	char *buffer = NULL;
+	autofree char *buffer = NULL;
 	int retval = 0, size = 0, asize = 0, rsize;
 	int chunksize = 1 << 14;
 	int fd;
@@ -637,7 +634,7 @@ int process_binary(int option, const char *profilename)
 
 	do {
 		if (asize - size == 0) {
-		  buffer = (char *) realloc(buffer, chunksize);
+			buffer = (char *) realloc(buffer, chunksize);
 			asize = chunksize;
 			chunksize <<= 1;
 			if (!buffer) {
@@ -658,8 +655,6 @@ int process_binary(int option, const char *profilename)
 	else
 		retval = rsize;
 
-	free(buffer);
-
 	if (conf_verbose) {
 		switch (option) {
 		case OPTION_ADD:
@@ -694,7 +689,7 @@ int test_for_dir_mode(const char *basename, const char *linkdir)
 	int rc = 0;
 
 	if (!skip_mode_force) {
-		char *target = NULL;
+		autofree char *target = NULL;
 		if (asprintf(&target, "%s/%s/%s", basedir, linkdir, basename) < 0) {
 			perror("asprintf");
 			exit(1);
@@ -702,8 +697,6 @@ int test_for_dir_mode(const char *basename, const char *linkdir)
 
 		if (access(target, R_OK) == 0)
 			rc = 1;
-
-		free(target);
 	}
 
 	return rc;
@@ -713,8 +706,8 @@ int process_profile(int option, const char *profilename)
 {
 	struct stat stat_bin;
 	int retval = 0;
-	char * cachename = NULL;
-	char * cachetemp = NULL;
+	autofree char *cachename = NULL;
+	autofree char *cachetemp = NULL;
 	const char *basename = NULL;
 
 	/* per-profile states */
@@ -879,10 +872,8 @@ out:
 			unlink(cachetemp);
 			PERROR("Warning failed to create cache: %s\n", basename);
 		}
-		free(cachetemp);
 	}
-	if (cachename)
-		free(cachename);
+
 	return retval;
 }
 
@@ -894,11 +885,10 @@ static int profile_dir_cb(DIR *dir unused, const char *name, struct stat *st,
 
 	if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) {
 		const char *dirname = (const char *)data;
-		char *path;
+		autofree char *path = NULL;
 		if (asprintf(&path, "%s/%s", dirname, name) < 0)
 			PERROR(_("Out of memory"));
 		rc = process_profile(option, path);
-		free(path);
 	}
 	return rc;
 }
@@ -911,19 +901,18 @@ static int binary_dir_cb(DIR *dir unused, const char *name, struct stat *st,
 
 	if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) {
 		const char *dirname = (const char *)data;
-		char *path;
+		autofree char *path = NULL;
 		if (asprintf(&path, "%s/%s", dirname, name) < 0)
 			PERROR(_("Out of memory"));
 		rc = process_binary(option, path);
-		free(path);
 	}
 	return rc;
 }
 
 static void setup_flags(void)
 {
-	char *cache_features_path = NULL;
-	char *cache_flags = NULL;
+	autofree char *cache_features_path = NULL;
+	autofree char *cache_flags = NULL;
 
 	/* Get the match string to determine type of regex support needed */
 	set_supported_features();
@@ -964,13 +953,9 @@ static void setup_flags(void)
 				skip_read_cache = 1;
 			}
 		}
-		free(cache_flags);
-		cache_flags = NULL;
 	} else if (write_cache) {
 		create_cache(cacheloc, cache_features_path, features_string);
 	}
-
-	free(cache_features_path);
 }
 
 int main(int argc, char *argv[])
-- 
2.1.4




More information about the AppArmor mailing list