On 2015-03-06 15:48:23, Tyler Hicks wrote:
> From: John Johansen <[email protected]>
> 
> 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 <[email protected]>

Acked-by: Tyler Hicks <[email protected]>


> ---
>  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
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

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