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;
>
>> ---
>> === modified file 'parser/parser_main.c'
>> --- parser/parser_main.c 2013-09-23 22:26:51 +0000
>> +++ parser/parser_main.c 2013-09-23 23:09:21 +0000
>> @@ -1260,20 +1260,42 @@
>> return error;
>> }
>>
>> -static int create_cache(const char *path, const char *features)
>> +static int create_cache(const char *cachedir, const char *path,
>> + const char *features)
>> {
>> + struct stat stat_file;
>> FILE * f = NULL;
>>
>> + if (cond_clear_cache && clear_cache_files(cacheloc) != 0)
>
> If --skip-bad-cache gets passed, then cond_clear_cache gets set to 0
> (false) so the goto error never occurs, instead it falls through to the
> create_file target, whereupon it creates the features file (and leaves
> write_cache set to 1, so it later writes out the cache file as well).
>
>> + goto error;
>> +
>> +create_file:
>> f = fopen(path, "w");
>> if (f) {
>> if (fwrite(features, strlen(features), 1, f) != 1 )
>> - goto fail;
>> + goto error;
>>
>> fclose(f);
>> - }
>> -
>> - return 0;
>> -fail:
>> +
>> +
>> + return 0;
>> + }
>> +
>> +error:
>> + /* does the dir exist? */
>> + if (stat(cachedir, &stat_file) == -1) {
>> + if (mkdir(cachedir, 0700) == 0)
>> + goto create_file;
>> + if (show_cache)
>> + PERROR(_("Can't create cache directory: %s\n"),
>> cachedir);
>> + } else if (!S_ISDIR(stat_file.st_mode)) {
>> + if (show_cache)
>> + PERROR(_("File in cache directory location: %s\n"),
>> cachedir);
>> + } else {
>> + if (show_cache)
>> + PERROR(_("Can't update cache directory: %s\n"),
>> cachedir);
>> + }
>> +
>> if (show_cache)
>> PERROR("Cache write disabled: cannot create %s\n", path);
>> write_cache = 0;
>
> This patch addresses the issue:
>
> Signed-off-by: Steve Beattie <[email protected]>
> ---
> parser/parser_main.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: b/parser/parser_main.c
> ===================================================================
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -1122,7 +1122,10 @@ static int create_cache(const char *cach
> struct stat stat_file;
> FILE * f = NULL;
>
> - if (cond_clear_cache && clear_cache_files(cacheloc) != 0)
> + if (!cond_clear_cache)
> + goto error;
> +
> + if (clear_cache_files(cacheloc) != 0)
> goto error;
>
> create_file:
>
>
>
>
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
