[apparmor] [PATCH v2 11/42] parser: Move policy cache functionality into policy_cache.c
Tyler Hicks
tyhicks at canonical.com
Tue Mar 24 21:02:35 UTC 2015
On 2015-03-06 15:48:27, Tyler Hicks wrote:
> 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>
> ---
Re-reviewed this one and it still looks good.
Tyler
> 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
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150324/f77df3b4/attachment.pgp>
More information about the AppArmor
mailing list