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 <[email protected]>
> > 
> > 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 <[email protected]>

-- 
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/

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