[apparmor] [PATCH v2 07/42] Use the gcc cleanup extension attribute to handle closing temp files
Tyler Hicks
tyhicks at canonical.com
Tue Mar 24 00:30:06 UTC 2015
On 2015-03-06 15:48:23, Tyler Hicks wrote:
> From: John Johansen <john.johansen at canonical.com>
>
> While some of these allocations will go away as we convert to C++,
> some of these need to stay C as the are going to be moved into a
> library to support loading cache from init daemons etc.
>
> For the bits that will eventually be C++ this helps clean things up,
> in the interim.
>
> TODO: apply to libapparmor as well
>
> Signed-off-by: John Johansen <john.johansen at canonical.com>
Acked-by: Tyler Hicks <tyhicks at canonical.com>
> ---
> parser/features.c | 9 +++------
> parser/lib.c | 17 +++++++++++++++++
> parser/lib.h | 4 ++++
> parser/network.c | 5 +++--
> parser/parser_include.c | 5 +++--
> parser/parser_interface.c | 12 +++---------
> parser/parser_main.c | 15 ++++-----------
> parser/policy_cache.c | 8 ++------
> 8 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/parser/features.c b/parser/features.c
> index d183365..673fe4f 100644
> --- a/parser/features.c
> +++ b/parser/features.c
> @@ -72,7 +72,8 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "%s {", name);
>
> if (S_ISREG(st->st_mode)) {
> - int len, file;
> + autoclose int file = -1;
> + int len;
> int remaining = fst->size - (fst->pos - *fst->buffer);
>
> file = openat(dirfd(dir), name, O_RDONLY);
> @@ -98,7 +99,6 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> PDEBUG("Error reading feature file '%s'\n", name);
> return -1;
> }
> - close(file);
> } else if (S_ISDIR(st->st_mode)) {
> if (dirat_for_each(dir, name, fst, features_dir_cb))
> return -1;
> @@ -124,7 +124,7 @@ static char *handle_features_dir(const char *filename, char **buffer, int size,
>
> char *load_features_file(const char *name) {
> char *buffer;
> - FILE *f = NULL;
> + autofclose FILE *f = NULL;
> size_t size;
>
> f = fopen(name, "r");
> @@ -140,14 +140,11 @@ char *load_features_file(const char *name) {
> goto fail;
> buffer[size] = 0;
>
> - fclose(f);
> return buffer;
>
> fail:
> int save = errno;
> free(buffer);
> - if (f)
> - fclose(f);
> errno = save;
> return NULL;
> }
> diff --git a/parser/lib.c b/parser/lib.c
> index fb9df49..5340971 100644
> --- a/parser/lib.c
> +++ b/parser/lib.c
> @@ -40,6 +40,23 @@ void __autofree(void *p)
> free(*_p);
> }
>
> +void __autoclose(int *fd)
> +{
> + if (*fd != -1) {
> + /* if close was interrupted retry */
> + while(close(*fd) == -1 && errno == EINTR);
> + *fd = -1;
> + }
> +}
> +
> +void __autofclose(FILE **f)
> +{
> + if (*f) {
> + fclose(*f);
> + *f = NULL;
> + }
> +}
> +
> /**
> * dirat_for_each: iterate over a directory calling cb for each entry
> * @dir: already opened directory (MAY BE NULL)
> diff --git a/parser/lib.h b/parser/lib.h
> index 73e1ffc..f047fd8 100644
> --- a/parser/lib.h
> +++ b/parser/lib.h
> @@ -4,7 +4,11 @@
> #include <dirent.h>
>
> #define autofree __attribute((cleanup(__autofree)))
> +#define autoclose __attribute((cleanup(__autoclose)))
> +#define autofclose __attribute((cleanup(__autofclose)))
> void __autofree(void *p);
> +void __autoclose(int *fd);
> +void __autofclose(FILE **f);
>
> int dirat_for_each(DIR *dir, const char *name, void *data,
> int (* cb)(DIR *, const char *, struct stat *, void *));
> diff --git a/parser/network.c b/parser/network.c
> index 80cce67..f26378b 100644
> --- a/parser/network.c
> +++ b/parser/network.c
> @@ -25,6 +25,7 @@
> #include <sstream>
> #include <map>
>
> +#include "lib.h"
> #include "parser.h"
> #include "profile.h"
> #include "parser_yacc.h"
> @@ -154,7 +155,8 @@ static struct network_tuple network_mappings[] = {
> static size_t kernel_af_max(void) {
> char buffer[32];
> int major;
> - int fd, res;
> + autoclose int fd = -1;
> + int res;
>
> if (!net_af_max_override) {
> return 0;
> @@ -168,7 +170,6 @@ static size_t kernel_af_max(void) {
> /* fall back to default provided during build */
> return 0;
> res = read(fd, &buffer, sizeof(buffer) - 1);
> - close(fd);
> if (res <= 0)
> return 0;
> buffer[res] = '\0';
> diff --git a/parser/parser_include.c b/parser/parser_include.c
> index 234d2d2..f4f1e23 100644
> --- a/parser/parser_include.c
> +++ b/parser/parser_include.c
> @@ -45,6 +45,8 @@
> #include <unistd.h>
> #include <errno.h>
> #include <dirent.h>
> +
> +#include "lib.h"
> #include "parser.h"
> #include "parser_include.h"
>
> @@ -176,7 +178,7 @@ int add_search_dir(const char *dir)
> SUBDOMAIN_PATH=/etc/subdomain.d/include */
> void parse_default_paths(void)
> {
> - FILE *f;
> + autofclose FILE *f;
> char buf[1024];
> char *t, *s;
> int saved_npath = npath;
> @@ -202,7 +204,6 @@ void parse_default_paths(void)
> } while (s != NULL);
> }
> }
> - fclose(f);
>
> /* if subdomain.conf doesn't set a base search dir set it to this */
> out:
> diff --git a/parser/parser_interface.c b/parser/parser_interface.c
> index 485c30b..7829b22 100644
> --- a/parser/parser_interface.c
> +++ b/parser/parser_interface.c
> @@ -479,7 +479,7 @@ void sd_serialize_top_profile(std::ostringstream &buf, Profile *profile)
> int cache_fd = -1;
> int __sd_serialize_profile(int option, Profile *prof)
> {
> - int fd = -1;
> + autoclose int fd = -1;
> int error = -ENOMEM, size, wsize;
> std::ostringstream work_area;
> autofree char *filename = NULL;
> @@ -594,9 +594,6 @@ int __sd_serialize_profile(int option, Profile *prof)
> }
> }
>
> - if (fd != -1)
> - close(fd);
> -
> if (!prof->hat_table.empty() && option != OPTION_REMOVE) {
> if (load_flattened_hats(prof, option) == 0)
> return 0;
> @@ -641,7 +638,7 @@ static int write_buffer(int fd, char *buffer, int size, bool set)
>
> int sd_load_buffer(int option, char *buffer, int size)
> {
> - int fd = -1;
> + autoclose int fd = -1;
> int error, bsize;
> autofree char *filename = NULL;
>
> @@ -666,8 +663,7 @@ int sd_load_buffer(int option, char *buffer, int size)
> if (fd < 0) {
> PERROR(_("Unable to open %s - %s\n"), filename,
> strerror(errno));
> - error = -errno;
> - goto out;
> + return -errno;
> }
>
> if (kernel_supports_setload) {
> @@ -688,8 +684,6 @@ int sd_load_buffer(int option, char *buffer, int size)
> break;
> }
> }
> - close(fd);
>
> -out:
> return error;
> }
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index f18d5f4..342ee14 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -498,7 +498,7 @@ static int process_args(int argc, char *argv[])
> static int process_config_file(const char *name)
> {
> char *optarg;
> - FILE *f;
> + autofclose FILE *f = NULL;
> int c, o;
>
> f = fopen(name, "r");
> @@ -507,7 +507,6 @@ static int process_config_file(const char *name)
>
> while ((c = getopt_long_file(f, long_options, &optarg, &o)) != -1)
> process_arg(c, optarg);
> - fclose(f);
> return 1;
> }
>
> @@ -553,7 +552,7 @@ int have_enough_privilege(void)
>
> static void set_features_by_match_file(void)
> {
> - FILE *ms = fopen(MATCH_FILE, "r");
> + autofclose FILE *ms = fopen(MATCH_FILE, "r");
> if (ms) {
> autofree char *match_string = (char *) malloc(1000);
> if (!match_string)
> @@ -563,14 +562,10 @@ static void set_features_by_match_file(void)
> if (strstr(match_string, " perms=c"))
> perms_create = 1;
> kernel_supports_network = 1;
> - goto out;
> + return;
> }
> no_match:
> perms_create = 1;
> -
> -out:
> - if (ms)
> - fclose(ms);
> }
>
> static void set_supported_features(void) {
> @@ -618,7 +613,7 @@ int process_binary(int option, const char *profilename)
> autofree char *buffer = NULL;
> int retval = 0, size = 0, asize = 0, rsize;
> int chunksize = 1 << 14;
> - int fd;
> + autoclose int fd = -1;
>
> if (profilename) {
> fd = open(profilename, O_RDONLY);
> @@ -648,8 +643,6 @@ int process_binary(int option, const char *profilename)
> size += rsize;
> } while (rsize > 0);
>
> - close(fd);
> -
> if (rsize == 0)
> retval = sd_load_buffer(option, buffer, size);
> else
> diff --git a/parser/policy_cache.c b/parser/policy_cache.c
> index f2909b9..b6f8299 100644
> --- a/parser/policy_cache.c
> +++ b/parser/policy_cache.c
> @@ -40,13 +40,12 @@ const char header_string[] = "\004\010\000version\000\002";
> bool valid_cached_file_version(const char *cachename)
> {
> char buffer[16];
> - FILE *f;
> + autofclose FILE *f;
> if (!(f = fopen(cachename, "r"))) {
> PERROR(_("Error: Could not read cache file '%s', skipping...\n"), cachename);
> return false;
> }
> size_t res = fread(buffer, 1, 16, f);
> - fclose(f);
> if (res < 16) {
> if (debug_cache)
> pwarn("%s: cache file '%s' invalid size\n", progname, cachename);
> @@ -111,7 +110,7 @@ int clear_cache_files(const char *path)
> int create_cache(const char *cachedir, const char *path, const char *features)
> {
> struct stat stat_file;
> - FILE * f = NULL;
> + autofclose FILE * f = NULL;
>
> if (clear_cache_files(cachedir) != 0)
> goto error;
> @@ -122,9 +121,6 @@ create_file:
> if (fwrite(features, strlen(features), 1, f) != 1 )
> goto error;
>
> - fclose(f);
> -
> -
> return 0;
> }
>
> --
> 2.1.4
>
>
> --
> 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: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150323/f829232d/attachment-0001.pgp>
More information about the AppArmor
mailing list