[apparmor] [PATCH 13/31] parser: Don't use the basedir global in setup_cache()

[John Johansen]

On 12/05/2014 04:22 PM, Tyler Hicks wrote:
> Require the caller of setup_cache() to pass in a valid cache location
> string. This removes the use of the basedir global from the
> policy_cache.c file.
> 
> Additionally, it is no longer necessary to return the "cache dir" path
> from setup_cache() since it will always be the same as the input path.
> The return value is changed to an int so an error code can be returned
> instead of using exit().
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  parser/parser_main.c  | 17 +++++++++++++----
>  parser/policy_cache.c | 30 +++++++++++-------------------
>  parser/policy_cache.h |  2 +-
>  3 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index 9056916..f1edd90 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -858,7 +858,6 @@ static void setup_flags(void)
>  
>  int main(int argc, char *argv[])
>  {
> -	autofree char *cachedir = NULL;
>  	int retval, last_error;
>  	int i;
>  	int optind;
> @@ -891,7 +890,17 @@ int main(int argc, char *argv[])
>  
>  	setup_flags();
>  
> -	cachedir = setup_cache(cacheloc);
> +	if (!cacheloc && asprintf(&cacheloc, "%s/cache", basedir) == -1) {
> +		PERROR(_("Memory allocation error."));
> +		return 1;
> +	}
> +
> +	retval = setup_cache(cacheloc);
> +	if (retval) {
> +		PERROR(_("Failed setting up policy cache (%s): %s\n"),
> +		       cacheloc, strerror(errno));
> +		return 1;
> +	}
>  
>  	retval = last_error = 0;
>  	for (i = optind; i <= argc; i++) {
> @@ -919,7 +928,7 @@ int main(int argc, char *argv[])
>  			struct dir_cb_data cb_data;
>  
>  			cb_data.dirname = profilename;
> -			cb_data.cachedir = cachedir;
> +			cb_data.cachedir = cacheloc;
>  			cb = binary_input ? binary_dir_cb : profile_dir_cb;
>  			if ((retval = dirat_for_each(NULL, profilename,
>  						     &cb_data, cb))) {
> @@ -929,7 +938,7 @@ int main(int argc, char *argv[])
>  		} else if (binary_input) {
>  			retval = process_binary(option, profilename);
>  		} else {
> -			retval = process_profile(option, profilename, cachedir);
> +			retval = process_profile(option, profilename, cacheloc);
>  		}
>  
>  		if (profilename) free(profilename);
> diff --git a/parser/policy_cache.c b/parser/policy_cache.c
> index b9aa795..e7c3c60 100644
> --- a/parser/policy_cache.c
> +++ b/parser/policy_cache.c
> @@ -228,27 +228,18 @@ void install_cache(const char *cachetmpname, const char *cachename)
>  	}
>  }
>  
> -char *setup_cache(const char *cacheloc)
> +int setup_cache(const char *cacheloc)
>  {
>  	autofree char *cache_features_path = NULL;
>  	autofree char *cache_flags = NULL;
> -	char *cachedir;
>  
> -	if (cacheloc) {
> -		cachedir = strdup(cacheloc);
> -		if (!cachedir) {
> -			PERROR(_("Memory allocation error."));
> -			exit(1);
> -		}
> -	} else {
> -		if (asprintf(&cachedir, "%s/cache", basedir) == -1) {
> -			PERROR(_("Memory allocation error."));
> -			exit(1);
> -		}
> +	if (!cacheloc) {
> +		errno = EINVAL;
> +		return -1;
>  	}
>  
>  	if (force_clear_cache)
> -		exit(clear_cache_files(cachedir));
> +		exit(clear_cache_files(cacheloc));
>  
>  	/*
>           * Deal with cache directory versioning:
> @@ -256,16 +247,17 @@ char *setup_cache(const char *cacheloc)
>           *  - If cache/.features exists, and does not match features_string,
>           *    force cache reading/writing off.
>           */
> -	if (asprintf(&cache_features_path, "%s/.features", cachedir) == -1) {
> +	if (asprintf(&cache_features_path, "%s/.features", cacheloc) == -1) {
>  		PERROR(_("Memory allocation error."));
> -		exit(1);
> +		errno = ENOMEM;
> +		return -1;
>  	}
>  
>  	cache_flags = load_features_file(cache_features_path);
>  	if (cache_flags) {
>  		if (strcmp(features_string, cache_flags) != 0) {
>  			if (write_cache && cond_clear_cache) {
> -				if (create_cache(cachedir, cache_features_path,
> +				if (create_cache(cacheloc, cache_features_path,
>  						 features_string))
>  					skip_read_cache = 1;
>  			} else {
> @@ -276,8 +268,8 @@ char *setup_cache(const char *cacheloc)
>  			}
>  		}
>  	} else if (write_cache) {
> -		create_cache(cachedir, cache_features_path, features_string);
> +		create_cache(cacheloc, cache_features_path, features_string);
>  	}
>  
> -	return cachedir;
> +	return 0;
>  }
> diff --git a/parser/policy_cache.h b/parser/policy_cache.h
> index 76d2f16..68d45a4 100644
> --- a/parser/policy_cache.h
> +++ b/parser/policy_cache.h
> @@ -46,6 +46,6 @@ 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);
> -char *setup_cache(const char *cacheloc);
> +int setup_cache(const char *cacheloc);
>  
>  #endif /* __AA_POLICY_CACHE_H */
>