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

[Tyler Hicks]

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 <john.johansen at canonical.com>

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

... 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
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- 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/20130927/67534d97/attachment.pgp>