[apparmor] [PATCH v2 11/42] parser: Move policy cache functionality into policy_cache.c

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


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

Signed-off-by: John Johansen <john.johansen at canonical.com>
[tyhicks: Fixed build failures]
[tyhicks: Fixed bug where a warning was being printed when it shouldn't]
[tyhicks: Forward ported to trunk]
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 parser/parser.h           |  6 ++--
 parser/parser_interface.c | 11 +++----
 parser/parser_main.c      | 84 +++++++++++------------------------------------
 parser/parser_policy.c    | 12 +++----
 parser/policy_cache.c     | 81 +++++++++++++++++++++++++++++++++++++++++++++
 parser/policy_cache.h     |  5 +++
 6 files changed, 119 insertions(+), 80 deletions(-)

diff --git a/parser/parser.h b/parser/parser.h
index 12b8b86..b9fc018 100644
--- a/parser/parser.h
+++ b/parser/parser.h
@@ -427,7 +427,7 @@ extern void free_aliases(void);
 extern int profile_merge_rules(Profile *prof);
 
 /* parser_interface.c */
-extern int load_profile(int option, Profile *prof);
+extern int load_profile(int option, Profile *prof, int cache_fd);
 extern void sd_serialize_profile(std::ostringstream &buf, Profile *prof,
 				int flatten);
 extern int sd_load_buffer(int option, char *buffer, int size);
@@ -448,9 +448,9 @@ extern int process_profile_policydb(Profile *prof);
 extern int post_merge_rules(void);
 extern int merge_hat_rules(Profile *prof);
 extern Profile *merge_policy(Profile *a, Profile *b);
-extern int load_policy(int option);
+extern int load_policy(int option, int cache_fd);
 extern int load_hats(std::ostringstream &buf, Profile *prof);
-extern int load_flattened_hats(Profile *prof, int option);
+extern int load_flattened_hats(Profile *prof, int option, int cache_fd);
 extern void dump_policy_hats(Profile *prof);
 extern void dump_policy_names(void);
 void dump_policy(void);
diff --git a/parser/parser_interface.c b/parser/parser_interface.c
index 6a9c918..be279b1 100644
--- a/parser/parser_interface.c
+++ b/parser/parser_interface.c
@@ -42,7 +42,7 @@
 #define SD_STR_LEN (sizeof(u16))
 
 
-int __sd_serialize_profile(int option, Profile *prof);
+int __sd_serialize_profile(int option, Profile *prof, int cache_fd);
 
 static void print_error(int error)
 {
@@ -83,13 +83,13 @@ static void print_error(int error)
 	}
 }
 
-int load_profile(int option, Profile *prof)
+int load_profile(int option, Profile *prof, int cache_fd)
 {
 	int retval = 0;
 	int error = 0;
 
 	PDEBUG("Serializing policy for %s.\n", prof->name);
-	retval = __sd_serialize_profile(option, prof);
+	retval = __sd_serialize_profile(option, prof, cache_fd);
 
 	if (retval < 0) {
 		error = retval;	/* yeah, we'll just report the last error */
@@ -475,8 +475,7 @@ void sd_serialize_top_profile(std::ostringstream &buf, Profile *profile)
 	sd_serialize_profile(buf, profile, profile->parent ? 1 : 0);
 }
 
-int cache_fd = -1;
-int __sd_serialize_profile(int option, Profile *prof)
+int __sd_serialize_profile(int option, Profile *prof, int cache_fd)
 {
 	autoclose int fd = -1;
 	int error = -ENOMEM, size, wsize;
@@ -555,7 +554,7 @@ int __sd_serialize_profile(int option, Profile *prof)
 	}
 
 	if (!prof->hat_table.empty() && option != OPTION_REMOVE) {
-		if (load_flattened_hats(prof, option) == 0)
+		if (load_flattened_hats(prof, option, cache_fd) == 0)
 			return 0;
 	}
 
diff --git a/parser/parser_main.c b/parser/parser_main.c
index 3358373..01ed0e2 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -695,10 +695,10 @@ int test_for_dir_mode(const char *basename, const char *linkdir)
 
 int process_profile(int option, const char *profilename)
 {
-	struct stat stat_bin;
 	int retval = 0;
-	autofree char *cachename = NULL;
-	autofree char *cachetemp = NULL;
+	autofree const char *cachename = NULL;
+	autofree const char *cachetmpname = NULL;
+	autoclose int cachetmp = -1;
 	const char *basename = NULL;
 
 	/* per-profile states */
@@ -737,24 +737,8 @@ int process_profile(int option, const char *profilename)
 
 		/* setup cachename and tstamp */
 		if (!force_complain && !skip_cache) {
-			if (asprintf(&cachename, "%s/%s", cacheloc, basename)<0) {
-				PERROR(_("Memory allocation error."));
-				exit(1);
-			}
-			if (!skip_read_cache) {
-				if (stat(cachename, &stat_bin) == 0 &&
-				    stat_bin.st_size > 0) {
-					if (valid_cached_file_version(cachename))
-						set_mru_tstamp(stat_bin.st_ctim);
-					else if (!cond_clear_cache)
-						write_cache = 0;
-				} else {
-					if (!cond_clear_cache)
-						write_cache = 0;
-					if (debug_cache)
-						pwarn("%s: Invalid or missing cache file '%s' (%s)\n", progname, cachename, strerror(errno));
-				}
-			}
+			cachename = cache_filename(cacheloc, basename);
+			valid_read_cache(cachename);
 		}
 
 	}
@@ -773,36 +757,20 @@ int process_profile(int option, const char *profilename)
 	 * TODO: Add support for caching profiles in an alternate namespace
 	 * TODO: Add support for embedded namespace defines if they aren't
 	 *       removed from the language.
+	 * TODO: test profile->ns NOT profile_ns (must be after parse)
 	 */
 	if (profile_ns)
 		skip_cache = 1;
 
-	/* Do secondary test to see if cached binary profile is good,
-	 * instead of checking against a presupplied list of files
-	 * use the timestamps from the files that were parsed.
-	 * Parsing the profile is slower that doing primary cache check
-	 * its still faster than doing full compilation
-	 */
 	if (cachename) {
 		/* Load a binary cache if it exists and is newest */
-		if (!mru_skip_cache) {
-			if (show_cache)
-				PERROR("Cache hit: %s\n", cachename);
+		if (cache_hit(cachename)) {
 			retval = process_binary(option, cachename);
 			if (!retval || skip_bad_cache_rebuild)
 				return retval;
 		}
-		if (write_cache) {
-			/* Otherwise, set up to save a cached copy */
-			if (asprintf(&cachetemp, "%s-XXXXXX", cachename)<0) {
-				perror("asprintf");
-				return ENOMEM;
-			}
-			if ( (cache_fd = mkstemp(cachetemp)) < 0) {
-				perror("mkstemp");
-				return ENOMEM;
-			}
-		}
+
+		cachetmp = setup_cache_tmp(&cachetmpname, cachename);
 	}
 
 	if (show_cache)
@@ -838,32 +806,18 @@ int process_profile(int option, const char *profilename)
 		goto out;
 	}
 
-	retval = load_policy(option);
-
-out:
-	if (cachetemp) {
-		/* Only install the generate cache file if it parsed correctly
-                   and did not have write/close errors */
-		int useable_cache = (cache_fd != -1 && retval == 0);
-		if (cache_fd != -1) {
-			if (close(cache_fd))
-				useable_cache = 0;
-			cache_fd = -1;
-		}
-
-		if (useable_cache) {
-			if (rename(cachetemp, cachename) < 0) {
-				pwarn("Warning failed to write cache: %s\n", cachename);
-				unlink(cachetemp);
-			}
-			else if (show_cache)
-				PERROR("Wrote cache: %s\n", cachename);
-		}
-		else {
-			unlink(cachetemp);
-			PERROR("Warning failed to create cache: %s\n", basename);
+	/* cache file generated by load_policy */
+	retval = load_policy(option, cachetmp);
+	if (retval == 0 && write_cache) {
+		if (cachetmp == -1) {
+			unlink(cachetmpname);
+			PERROR("Warning failed to create cache: %s\n",
+			       basename);
+		} else {
+			install_cache(cachetmpname, cachename);
 		}
 	}
+out:
 
 	return retval;
 }
diff --git a/parser/parser_policy.c b/parser/parser_policy.c
index 966044d..137f9b6 100644
--- a/parser/parser_policy.c
+++ b/parser/parser_policy.c
@@ -220,12 +220,12 @@ static int profile_add_hat_rules(Profile *prof)
 	return 0;
 }
 
-int load_policy_list(ProfileList &list, int option)
+int load_policy_list(ProfileList &list, int option, int cache_fd)
 {
 	int res = 0;
 
 	for (ProfileList::iterator i = list.begin(); i != list.end(); i++) {
-		res = load_profile(option, *i);
+		res = load_profile(option, *i, cache_fd);
 		if (res != 0)
 			break;
 	}
@@ -233,14 +233,14 @@ int load_policy_list(ProfileList &list, int option)
 	return res;
 }
 
-int load_flattened_hats(Profile *prof, int option)
+int load_flattened_hats(Profile *prof, int option, int cache_fd)
 {
-	return load_policy_list(prof->hat_table, option);
+	return load_policy_list(prof->hat_table, option, cache_fd);
 }
 
-int load_policy(int option)
+int load_policy(int option, int cache_fd)
 {
-	return load_policy_list(policy_list, option);
+	return load_policy_list(policy_list, option, cache_fd);
 }
 
 int load_hats(std::ostringstream &buf, Profile *prof)
diff --git a/parser/policy_cache.c b/parser/policy_cache.c
index b6f8299..7d9818a 100644
--- a/parser/policy_cache.c
+++ b/parser/policy_cache.c
@@ -145,3 +145,84 @@ error:
 
 	return -1;
 }
+
+char *cache_filename(const char *cacheloc, const char *basename)
+{
+	char *cachename;
+
+	if (asprintf(&cachename, "%s/%s", cacheloc, basename) < 0) {
+		PERROR(_("Memory allocation error."));
+		exit(1);
+	}
+
+	return cachename;
+}
+
+void valid_read_cache(const char *cachename)
+{
+	struct stat stat_bin;
+
+	/* Load a binary cache if it exists and is newest */
+	if (!skip_read_cache) {
+		if (stat(cachename, &stat_bin) == 0 &&
+		    stat_bin.st_size > 0) {
+			if (valid_cached_file_version(cachename))
+				set_mru_tstamp(stat_bin.st_ctim);
+			else if (!cond_clear_cache)
+				write_cache = 0;
+		} else {
+			if (!cond_clear_cache)
+				write_cache = 0;
+			if (debug_cache)
+				pwarn("%s: Invalid or missing cache file '%s' (%s)\n", progname, cachename, strerror(errno));
+		}
+	}
+}
+
+int cache_hit(const char *cachename)
+{
+	if (!mru_skip_cache) {
+		if (show_cache)
+			PERROR("Cache hit: %s\n", cachename);
+		return true;
+	}
+
+	return false;
+}
+
+int setup_cache_tmp(const char **cachetmpname, const char *cachename)
+{
+	char *tmpname;
+	int cache_fd = -1;
+
+	*cachetmpname = NULL;
+	if (write_cache) {
+		/* Otherwise, set up to save a cached copy */
+		if (asprintf(&tmpname, "%s-XXXXXX", cachename)<0) {
+			perror("asprintf");
+			exit(1);
+		}
+		if ((cache_fd = mkstemp(tmpname)) < 0) {
+			perror("mkstemp");
+			exit(1);
+		}
+		*cachetmpname = tmpname;
+	}
+
+	return cache_fd;
+}
+
+void install_cache(const char *cachetmpname, const char *cachename)
+{
+	/* Only install the generate cache file if it parsed correctly
+	   and did not have write/close errors */
+	if (cachetmpname) {
+		if (rename(cachetmpname, cachename) < 0) {
+			pwarn("Warning failed to write cache: %s\n", cachename);
+			unlink(cachetmpname);
+		}
+		else if (show_cache) {
+			PERROR("Wrote cache: %s\n", cachename);
+		}
+	}
+}
diff --git a/parser/policy_cache.h b/parser/policy_cache.h
index 2b9a5dc..8572ff6 100644
--- a/parser/policy_cache.h
+++ b/parser/policy_cache.h
@@ -41,5 +41,10 @@ void update_mru_tstamp(FILE *file, const char *path);
 bool valid_cached_file_version(const char *cachename);
 int clear_cache_files(const char *path);
 int create_cache(const char *cachedir, const char *path, const char *features);
+char *cache_filename(const char *cacheloc, const char *basename);
+void valid_read_cache(const char *cachename);
+int cache_hit(const char *cachename);
+int setup_cache_tmp(const char **cachetmpname, const char *cachename);
+void install_cache(const char *cachetmpname, const char *cachename);
 
 #endif /* __AA_POLICY_CACHE_H */
-- 
2.1.4




More information about the AppArmor mailing list