[apparmor] [patch] [parser]: create missing cache directory

Steve Beattie steve at nxnw.org
Thu Oct 3 21:00:24 UTC 2013


On Thu, Oct 03, 2013 at 01:40:50PM -0700, John Johansen wrote:
> On 10/03/2013 09:20 AM, Steve Beattie wrote:
> > On Mon, Sep 23, 2013 at 04:13:49PM -0700, John Johansen wrote:
> >> This patch applies on top of the previous 2 cache patches. It does two
> >> things, create the cache dir if it is missing, and moves the cache clearing
> >> logic into the create cache routine, because if we are writing a new
> >> cache .features file the cache dir should be cleared out.
> >>
> >> Signed-off-by: John Johansen <john.johansen at canonical.com>
> > 
> > As near as I can tell, this patch breaks the '--skip-bad-cache'
> > argument, but I can't really tell for sure because I'm unclear on
> > what that argument's purpose is.
> > 
> its whether the cache gets cleared when an error is detected, so
> the inverse of the cond_clear_cache variable (don't ask me why I don't
> know/can't remember).
> 
> clearing the cache if doesn't match is done by default, but if you
> don't want that behavior you specify --skip-bad-cache
> 
> > In any event, this breakage can be seen by running the existing
> > tst/caching.sh script; after this commit is applied, the test:
> > 
> >   echo -n "Cache writing is skipped when features do not match and not cleared: "
> >   rm $basedir/cache/$profile
> >   ${APPARMOR_PARSER} $ARGS -v --write-cache --skip-bad-cache -r $basedir/$profile | grep -q 'Replacement succeeded for' || { echo "FAIL"; exit 1; }
> >   [ -f $basedir/cache/$profile ] && echo "FAIL ($basedir/cache/$profile exists)" && exit 1
> >   echo "ok"
> > 
> > fails, because the cached profile gets created.
> 
> yeah not sure why I didn't catch that, I did run the tests. I need to
> revert part of this patch with the following patch
> 
> 
> @@ -1122,7 +1164,7 @@
>  	struct stat stat_file;
>  	FILE * f = NULL;
>  
> -	if (cond_clear_cache && clear_cache_files(cacheloc) != 0)
> +	if (clear_cache_files(cacheloc) != 0)
>  		goto error;
>  
>  create_file:
> @@ -1197,7 +1239,7 @@
>  	get_flags_string(&cache_flags, cache_features_path);
>  	if (cache_flags) {
>  		if (strcmp(flags_string, cache_flags) != 0) {
> -			if (write_cache) {
> +			if (write_cache && cond_clear_cache) {
>  				if (create_cache(cacheloc, cache_features_path,
>  						 flags_string))
>  					skip_read_cache = 1;

Sure, that works as well as the patch I proposed.
Acked-by: Steve Beattie <steve at nxnw.org>

-- 
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: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20131003/ac5770e2/attachment.pgp>


More information about the AppArmor mailing list