On 2013-09-23 16:13:49, 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]>
Acked-by: Tyler Hicks <[email protected]> ... with the disclaimer that I'm not real clear on how the parser does its caching. It would be nice if you got an ack from someone else that understands this better than I do. However, if you feel confident in this patch, it looks correct to me and I think it would be fine to commit to trunk as-is. As a side note, should we be enforcing cache dir permissions? This patch creates the dir with a mode of 700. Do we typically expect distro packaging to create this dir? If so, maybe we should check their work... Tyler > > --- > === 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) > + 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; > @@ -1319,12 +1341,10 @@ > get_flags_string(&cache_flags, cache_features_path); > if (cache_flags) { > if (strcmp(flags_string, cache_flags) != 0) { > - if (write_cache && cond_clear_cache) { > - if (clear_cache_files(cacheloc) || > - create_cache(cache_features_path, > - flags_string)) { > + if (write_cache) { > + if (create_cache(cacheloc, cache_features_path, > + flags_string)) > skip_read_cache = 1; > - } > } else { > if (show_cache) > PERROR("Cache read/write disabled: %s > does not match %s\n", FLAGS_FILE, cache_features_path); > @@ -1335,7 +1355,7 @@ > free(cache_flags); > cache_flags = NULL; > } else if (write_cache) { > - create_cache(cache_features_path, flags_string); > + create_cache(cacheloc, cache_features_path, flags_string); > } > > free(cache_features_path); > > > > -- > AppArmor mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
