[apparmor] [PATCH 2/6] libapparmor: Rename aa_policy_cache_create

Steve Beattie steve at nxnw.org
Thu Mar 26 06:46:26 UTC 2015


On Wed, Mar 25, 2015 at 05:37:17PM -0500, Tyler Hicks wrote:
> The aa_policy_cache_create() function had a name that didn't quite match
> its actions. It doesn't create a new policy cache. It actually requires
> an existing policy cache, with some sort of .features file, to already
> exist.
> 
> It unconditionally makes a policy cache "valid" by clearing all of the
> files and creating a new .features file from the current running kernel.

Given that, perhaps aa_policy_cache_reset() + a lavender bikeshed? Also,
some manpages would be nice to document the purpose of each function
and any sequencing issues.

That said, in the end, either works for me, so

> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

Acked-by: Steve Beattie <steve at nxnw.org> with your choice on keeping
'make_valid' or converting to 'reset' (with my slight preference for
reset).

Thanks.

> ---
>  libraries/libapparmor/include/sys/apparmor.h |  2 +-
>  libraries/libapparmor/src/libapparmor.map    |  2 +-
>  libraries/libapparmor/src/policy_cache.c     |  4 ++--
>  parser/parser_main.c                         |  2 +-
>  tests/regression/apparmor/aa_policy_cache.c  | 14 +++++++-------
>  tests/regression/apparmor/aa_policy_cache.sh |  8 ++++----
>  6 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/libraries/libapparmor/include/sys/apparmor.h b/libraries/libapparmor/include/sys/apparmor.h
> index 99ce36b..43d6efc 100644
> --- a/libraries/libapparmor/include/sys/apparmor.h
> +++ b/libraries/libapparmor/include/sys/apparmor.h
> @@ -147,7 +147,7 @@ aa_policy_cache *aa_policy_cache_ref(aa_policy_cache *policy_cache);
>  void aa_policy_cache_unref(aa_policy_cache *policy_cache);
>  
>  bool aa_policy_cache_is_valid(aa_policy_cache *policy_cache);
> -int aa_policy_cache_create(aa_policy_cache *policy_cache);
> +int aa_policy_cache_make_valid(aa_policy_cache *policy_cache);
>  int aa_policy_cache_remove(const char *path);
>  int aa_policy_cache_replace_all(aa_policy_cache *policy_cache,
>  				aa_kernel_interface *kernel_interface);
> diff --git a/libraries/libapparmor/src/libapparmor.map b/libraries/libapparmor/src/libapparmor.map
> index 3f43494..2f440f0 100644
> --- a/libraries/libapparmor/src/libapparmor.map
> +++ b/libraries/libapparmor/src/libapparmor.map
> @@ -77,7 +77,7 @@ APPARMOR_2.10 {
>          aa_policy_cache_ref;
>          aa_policy_cache_unref;
>          aa_policy_cache_is_valid;
> -        aa_policy_cache_create;
> +        aa_policy_cache_make_valid;
>          aa_policy_cache_remove;
>          aa_policy_cache_replace_all;
>    local:
> diff --git a/libraries/libapparmor/src/policy_cache.c b/libraries/libapparmor/src/policy_cache.c
> index a9e43bb..e438439 100644
> --- a/libraries/libapparmor/src/policy_cache.c
> +++ b/libraries/libapparmor/src/policy_cache.c
> @@ -225,13 +225,13 @@ bool aa_policy_cache_is_valid(aa_policy_cache *policy_cache)
>  }
>  
>  /**
> - * aa_policy_cache_create - creates a valid policy_cache for the currently running kernel
> + * aa_policy_cache_make_valid - empties the policy_cache and makes it valid for the currently running kernel
>   * @policy_cache: the policy_cache
>   *
>   * Returns: 0 on success, -1 on error with errno set and features pointing to
>   *          NULL
>   */
> -int aa_policy_cache_create(aa_policy_cache *policy_cache)
> +int aa_policy_cache_make_valid(aa_policy_cache *policy_cache)
>  {
>  	return create_cache(policy_cache, policy_cache->kernel_features);
>  }
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index 8aee148..1dc3088 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -929,7 +929,7 @@ int main(int argc, char *argv[])
>  			skip_read_cache = 0;
>  		} else if (!aa_policy_cache_is_valid(policy_cache)) {
>  			if (write_cache && cond_clear_cache &&
> -			    aa_policy_cache_create(policy_cache)) {
> +			    aa_policy_cache_make_valid(policy_cache)) {
>  				if (show_cache)
>  					PERROR("Cache write disabled: Cannot create cache '%s': %m\n",
>  					       cacheloc);
> diff --git a/tests/regression/apparmor/aa_policy_cache.c b/tests/regression/apparmor/aa_policy_cache.c
> index b08fd1f..cb4bc71 100644
> --- a/tests/regression/apparmor/aa_policy_cache.c
> +++ b/tests/regression/apparmor/aa_policy_cache.c
> @@ -22,7 +22,7 @@
>  
>  #include <sys/apparmor.h>
>  
> -#define OPT_CREATE		"create"
> +#define OPT_MAKE_VALID		"make-valid"
>  #define OPT_IS_VALID		"is-valid"
>  #define OPT_NEW			"new"
>  #define OPT_NEW_CREATE		"new-create"
> @@ -40,12 +40,12 @@ static void usage(const char *prog)
>  		"              %s %s <PATH>\n"
>  		"              %s %s <PROFILE_NAME>\n"
>  		"              %s %s <PATH>\n",
> -		prog, OPT_CREATE, prog, OPT_IS_VALID, prog, OPT_NEW,
> +		prog, OPT_MAKE_VALID, prog, OPT_IS_VALID, prog, OPT_NEW,
>  		prog, OPT_NEW_CREATE, prog, OPT_REMOVE, prog, OPT_REMOVE_POLICY,
>  		prog, OPT_REPLACE_ALL);
>  }
>  
> -static int test_create(const char *path)
> +static int test_make_valid(const char *path)
>  {
>  	aa_features *features = NULL;
>  	aa_policy_cache *policy_cache = NULL;
> @@ -61,8 +61,8 @@ static int test_create(const char *path)
>  		goto out;
>  	}
>  
> -	if (aa_policy_cache_create(policy_cache)) {
> -		perror("FAIL - aa_policy_cache_create");
> +	if (aa_policy_cache_make_valid(policy_cache)) {
> +		perror("FAIL - aa_policy_cache_make_valid");
>  		goto out;
>  	}
>  
> @@ -204,8 +204,8 @@ int main(int argc, char **argv)
>  		exit(1);
>  	}
>  
> -	if (strcmp(argv[1], OPT_CREATE) == 0) {
> -		rc = test_create(argv[2]);
> +	if (strcmp(argv[1], OPT_MAKE_VALID) == 0) {
> +		rc = test_make_valid(argv[2]);
>  	} else if (strcmp(argv[1], OPT_IS_VALID) == 0) {
>  		rc = test_is_valid(argv[2]);
>  	} else if (strcmp(argv[1], OPT_NEW) == 0) {
> diff --git a/tests/regression/apparmor/aa_policy_cache.sh b/tests/regression/apparmor/aa_policy_cache.sh
> index fb9a830..427ddfa 100755
> --- a/tests/regression/apparmor/aa_policy_cache.sh
> +++ b/tests/regression/apparmor/aa_policy_cache.sh
> @@ -117,12 +117,12 @@ runchecktest "AA_POLICY_CACHE is-valid (no cachedir)" fail is-valid "$cachedir"
>  
>  create_cachedir
>  install_bad_features_file
> -runchecktest "AA_POLICY_CACHE create (bad .features)" pass create "$cachedir"
> -runchecktest "AA_POLICY_CACHE create (good .features)" pass create "$cachedir"
> +runchecktest "AA_POLICY_CACHE make-valid (bad .features)" pass make-valid "$cachedir"
> +runchecktest "AA_POLICY_CACHE make-valid (good .features)" pass make-valid "$cachedir"
>  remove_features_file
> -runchecktest "AA_POLICY_CACHE create (no .features)" fail create "$cachedir"
> +runchecktest "AA_POLICY_CACHE make-valid (no .features)" fail make-valid "$cachedir"
>  remove_cachedir
> -runchecktest "AA_POLICY_CACHE create (no cachedir)" fail create "$cachedir"
> +runchecktest "AA_POLICY_CACHE make-valid (no cachedir)" fail make-valid "$cachedir"
>  
>  # Make sure that no test policies are already loaded
>  verify_policies_are_not_loaded

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- 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/20150325/134921db/attachment-0001.pgp>


More information about the AppArmor mailing list