On 2015-03-06 15:48:27, Tyler Hicks wrote:
> From: John Johansen <[email protected]>
> 
> Signed-off-by: John Johansen <[email protected]>
> [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 <[email protected]>
> ---

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
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to