"Move constructors" for cleaner resource handling

Change-Id: Icbfd6f8e7ccf59060b63a7a73d00eedee74e56ea

diff --git a/include/log_file.h b/include/log_file.h
index 7d09059..a6338c7 100644 (file)
--- a/include/log_file.h
+++ b/include/log_file.h
@@ -49,6 +49,7 @@ extern "C" {
 #endif
 
 void logfile_init(struct log_file *l_file);
+void logfile_move(struct log_file *to, struct log_file *from);
 void logfile_free(struct log_file *l_file);
 void logfile_set_fd(struct log_file *l_file, int fd, int should_close);
 int logfile_set_path(struct log_file *l_file, const char *path);

diff --git a/include/logprint.h b/include/logprint.h
index 6311e2d..072f12a 100644 (file)
--- a/include/logprint.h
+++ b/include/logprint.h
@@ -62,6 +62,8 @@ void log_filter_free(dlogutil_filter_options_s *p_filter);
  */
 dlogutil_filter_options_s *log_filter_from_filter(const dlogutil_filter_options_s *p_filter);
 
+dlogutil_filter_options_s *log_filter_move(dlogutil_filter_options_s *p_filter);
+
 /**
  * Returns FORMAT_OFF on invalid string
  */

diff --git a/src/logger/logger.c b/src/logger/logger.c
index 1e9c156..7bc1664 100644 (file)
--- a/src/logger/logger.c
+++ b/src/logger/logger.c
@@ -1031,17 +1031,17 @@ failure:
        return ret;
 }
 
-static struct reader_pipe *reader_pipe_alloc(dlogutil_filter_options_s *filter, struct log_file file, struct timespec ts,
+static struct reader_pipe *reader_pipe_alloc(dlogutil_filter_options_s *filter, struct log_file *file, struct timespec ts,
        bool monitor, bool is_dumping)
 {
        struct reader_pipe *ret = calloc(1, sizeof(*ret));
        if (!ret)
                return NULL;
 
-       ret->filter = filter;
+       ret->filter = log_filter_move(filter);
        ret->monitor = monitor;
        ret->is_dumping = is_dumping;
-       ret->file = file;
+       logfile_move(&ret->file, file);
        ret->buf_ptr = NULL;
        ret->log_storage_reader = NULL;
        ret->last_read_time = ts;
@@ -1050,7 +1050,7 @@ static struct reader_pipe *reader_pipe_alloc(dlogutil_filter_options_s *filter,
        return ret;
 }
 
-static struct reader_logger *reader_logger_alloc(dlogutil_filter_options_s *filter, struct log_file file)
+static struct reader_logger *reader_logger_alloc(dlogutil_filter_options_s *filter, struct log_file *file)
 {
        struct reader_logger *const ret = calloc(1, sizeof *ret);
        struct subreader_logger *const srl = malloc(sizeof *srl);
@@ -1062,12 +1062,12 @@ static struct reader_logger *reader_logger_alloc(dlogutil_filter_options_s *filt
                return NULL;
        }
 
-       srlf->file = file;
+       logfile_move(&srlf->file, file);
 
        srl->sub_userdata = srlf;
        srl->sub_destroy = subreader_logger_file_free;
        srl->sub_apply_log = subreader_logger_file_apply_log;
-       srl->filter = filter;
+       srl->filter = log_filter_move(filter);
 
        list_add(&ret->subs, srl);
 
@@ -1077,7 +1077,7 @@ static struct reader_logger *reader_logger_alloc(dlogutil_filter_options_s *filt
 }
 
 static int reader_pipe_init(struct reader_pipe **reader, log_id_t buf_id, struct logger *server,
-       dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping)
+       dlogutil_filter_options_s *filter, struct log_file *file, bool monitor, bool is_dumping)
 {
        assert(reader);
        assert(buf_id > LOG_ID_INVALID);
@@ -1102,7 +1102,7 @@ static int reader_pipe_init(struct reader_pipe **reader, log_id_t buf_id, struct
 }
 
 static int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writer *writer, struct logger *server,
-       dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping)
+       dlogutil_filter_options_s *filter, struct log_file *file, bool monitor, bool is_dumping)
 {
        assert(reader);
        assert(writer);
@@ -1125,7 +1125,7 @@ static int reader_pipe_init_with_writer(struct reader_pipe **reader, struct writ
 }
 
 static int reader_logger_init(struct reader_logger **reader, log_id_t buf_id, struct logger *server,
-       dlogutil_filter_options_s *filter, struct log_file file, bool monitor, bool is_dumping)
+       dlogutil_filter_options_s *filter, struct log_file *file, bool monitor, bool is_dumping)
 {
        assert(reader);
        assert(buf_id > LOG_ID_INVALID);
@@ -1249,6 +1249,13 @@ static bool initialize_dlogutil_line_params(struct dlogutil_line_params *params)
        return true;
 }
 
+static void free_dlogutil_line_params(struct dlogutil_line_params *params)
+{
+       logfile_free(&params->file);
+       log_filter_free(params->filter);
+       free(params->file_path);
+}
+
 static int make_argc_argv_from_dlogutil_line(const char *cmdl, size_t buf_size, char buffer[buf_size], int *argc, char **argv)
 {
        assert(cmdl);
@@ -1367,9 +1374,6 @@ static int get_dlogutil_params_from_argc_argv(int argc, char **argv, struct dlog
 static int get_dlogutil_line_params(const char *cmdl, struct dlogutil_line_params *params)
 {
        assert(params);
-       if (!initialize_dlogutil_line_params(params))
-               return -ENOMEM;
-
        assert(cmdl);
        char buffer[1024];
        char *argv[64]; // size arbitrary, should reasonably fit all args
@@ -1381,7 +1385,7 @@ static int get_dlogutil_line_params(const char *cmdl, struct dlogutil_line_param
        return get_dlogutil_params_from_argc_argv(argc, argv, params);
 }
 
-static int create_reader_logger_from_dlogutil_line(struct dlogutil_line_params params, struct logger *server, struct reader_logger **rd)
+static int create_reader_logger_from_dlogutil_line(struct dlogutil_line_params *params, struct logger *server, struct reader_logger **rd)
 {
        assert(server);
        assert(rd);
@@ -1389,50 +1393,36 @@ static int create_reader_logger_from_dlogutil_line(struct dlogutil_line_params p
        __attribute__((cleanup(reader_logger_cleanup))) struct reader_logger *reader = NULL;
        int retval;
 
-       if (params.file_path) {
-               retval = logfile_set_path(&params.file, params.file_path);
+       if (params->file_path) {
+               retval = logfile_set_path(&params->file, params->file_path);
                if (retval < 0)
-                       goto cleanup;
+                       return retval;
 
-               retval = logfile_open(&params.file);
+               retval = logfile_open(&params->file);
                if (retval < 0)
-                       goto cleanup;
+                       return retval;
        }
 
-       if (params.buf_id == LOG_ID_INVALID) {
-               retval = -EINVAL;
-               goto cleanup;
-       }
+       if (params->buf_id == LOG_ID_INVALID)
+               return -EINVAL;
 
-       retval = reader_logger_init(&reader, params.buf_id, server, params.filter, params.file, params.monitor, params.is_dumping);
-       if (retval != 0)
-               goto cleanup;
+       if (params->file.path == NULL)
+               return -EINVAL;
 
-       if (reader->common.fd_entity_source.fd < 0) {
-               retval = -EINVAL;
-               goto cleanup;
-       }
+       retval = reader_logger_init(&reader, params->buf_id, server, params->filter, &params->file, params->monitor, params->is_dumping);
+       if (retval != 0)
+               return retval;
 
-       if (params.file.path == NULL) {
-               retval = -EINVAL;
-               goto cleanup;
-       }
+       if (reader->common.fd_entity_source.fd < 0)
+               return -EINVAL;
 
        *rd = reader;
        reader = NULL;
-       return 0;
 
-cleanup:
-       if (!reader) {
-               // TODO: Free this in a reasonable way
-               logfile_free(&params.file);
-               log_filter_free(params.filter);
-       }
-       free(params.file_path);
-       return retval;
+       return 0;
 }
 
-static int create_reader_pipe_from_dlogutil_line(struct dlogutil_line_params params, struct logger *server, struct reader_pipe **rd)
+static int create_reader_pipe_from_dlogutil_line(struct dlogutil_line_params *params, struct logger *server, struct reader_pipe **rd)
 {
        assert(server);
        assert(rd);
@@ -1440,42 +1430,30 @@ static int create_reader_pipe_from_dlogutil_line(struct dlogutil_line_params par
        __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *reader = NULL;
        int retval;
 
-       if (params.file_path) {
-               retval = logfile_set_path(&params.file, params.file_path);
+       if (params->file_path) {
+               retval = logfile_set_path(&params->file, params->file_path);
                if (retval < 0)
-                       goto cleanup;
+                       return retval;
 
-               retval = logfile_open(&params.file);
+               retval = logfile_open(&params->file);
                if (retval < 0)
-                       goto cleanup;
+                       return retval;
        }
 
-       if (params.buf_id == LOG_ID_INVALID) {
-               retval = -EINVAL;
-               goto cleanup;
-       }
+       if (params->buf_id == LOG_ID_INVALID)
+               return -EINVAL;
 
-       retval = reader_pipe_init(&reader, params.buf_id, server, params.filter, params.file, params.monitor, params.is_dumping);
-       if (retval != 0)
-               goto cleanup;
+       if (params->file.path == NULL)
+               return -EINVAL;
 
-       if (params.file.path == NULL) {
-               retval = -EINVAL;
-               goto cleanup;
-       }
+       retval = reader_pipe_init(&reader, params->buf_id, server, params->filter, &params->file, params->monitor, params->is_dumping);
+       if (retval != 0)
+               return retval;
 
        *rd = reader;
        reader = NULL;
-       return 0;
 
-cleanup:
-       if (!reader) {
-               // TODO: Free this in a reasonable way
-               logfile_free(&params.file);
-               log_filter_free(params.filter);
-       }
-       free(params.file_path);
-       return retval;
+       return 0;
 }
 
 /**
@@ -1508,10 +1486,18 @@ static int service_writer_handle_req_util(struct logger* server, struct writer*
 
        __attribute__((cleanup(reader_pipe_cleanup))) struct reader_pipe *reader = NULL;
 
-       struct dlogutil_line_params params;
-       int retval = get_dlogutil_line_params(msg->data, &params);
-       if (retval < 0)
+       int retval;
+       __attribute__((cleanup(free_dlogutil_line_params))) struct dlogutil_line_params params;
+       if (!initialize_dlogutil_line_params(&params)) {
+               retval = -ENOMEM;
+               goto cleanup;
+       }
+
+       retval = get_dlogutil_line_params(msg->data, &params);
+       if (retval < 0) {
+               retval = -ENOMEM;
                goto cleanup;
+       }
 
        if (params.file_path) {
                /* Do not trust writer-based readers (only config-based).
@@ -1527,7 +1513,7 @@ static int service_writer_handle_req_util(struct logger* server, struct writer*
                goto cleanup;
        }
 
-       retval = reader_pipe_init_with_writer(&reader, wr, server, params.filter, params.file, params.monitor, params.is_dumping);
+       retval = reader_pipe_init_with_writer(&reader, wr, server, params.filter, &params.file, params.monitor, params.is_dumping);
        if (retval != 0)
                goto cleanup;
 
@@ -1555,13 +1541,6 @@ cleanup:
        if (retval < 0)
                printf("ERROR: both create_reader_from_dlogutil_line() and send_dlog_reply() failed\n");
 
-       // TODO: Free this in a reasonable way
-       if (!reader) {
-               logfile_free(&params.file);
-               log_filter_free(params.filter);
-       }
-       free(params.file_path);
-
        return retval;
 }
 
@@ -2589,21 +2568,21 @@ void parse_logfile_config(void *value, void *userdata)
        struct logger *server = (struct logger *) userdata;
        char *configline = (char *) value;
 
-       struct dlogutil_line_params params;
+       __attribute__((cleanup(free_dlogutil_line_params))) struct dlogutil_line_params params;
+       if (!initialize_dlogutil_line_params(&params))
+               return;
+
        get_dlogutil_line_params(configline, &params);
 
        int r;
        if (g_backend.use_logger_by_default && params.buf_id != LOG_ID_KMSG && params.buf_id != LOG_ID_SYSLOG) {
                struct reader_logger *reader = NULL;
-               r = create_reader_logger_from_dlogutil_line(params, server, &reader);
-               params = (struct dlogutil_line_params){}; // We no longer can use this struct
+               r = create_reader_logger_from_dlogutil_line(&params, server, &reader);
                if (r == 0)
                        add_reader_logger(server, reader);
-       }
-       else {
+       } else {
                struct reader_pipe *reader = NULL;
-               r = create_reader_pipe_from_dlogutil_line(params, server, &reader);
-               params = (struct dlogutil_line_params){}; // We no longer can use this struct
+               r = create_reader_pipe_from_dlogutil_line(&params, server, &reader);
                if (r == 0)
                        add_reader_pipe(server, reader);
        }

diff --git a/src/shared/log_file.c b/src/shared/log_file.c
index 92bb3ea..19e7fc1 100644 (file)
--- a/src/shared/log_file.c
+++ b/src/shared/log_file.c
@@ -137,6 +137,14 @@ static int logfile_open_internal(struct log_file *l_file)
        return 0;
 }
 
+void logfile_move(struct log_file *to, struct log_file *from)
+{
+       *to = *from;
+
+       from->path = NULL;
+       from->fd = -1;
+}
+
 /**
  * @brief Open a log file
  * @details Open a log file into the passed structure.

diff --git a/src/shared/logprint.c b/src/shared/logprint.c
index ccce864..2105fb6 100644 (file)
--- a/src/shared/logprint.c
+++ b/src/shared/logprint.c
@@ -250,6 +250,25 @@ dlogutil_filter_options_s *log_filter_from_filter(const dlogutil_filter_options_
 }
 
 /**
+ * @brief Move a log format
+ * @details Moves a log format (like C++ std::move)
+ * @param[in] p_format The log format to move
+ * @return The new structure (or NULL if allocation failed)
+ * @see log_filter_free
+ */
+dlogutil_filter_options_s *log_filter_move(dlogutil_filter_options_s *p_filter)
+{
+       dlogutil_filter_options_s *const ret = malloc(sizeof *ret);
+       if (!ret)
+               return NULL;
+
+       *ret = *p_filter;
+       p_filter->filters = NULL;
+
+       return ret;
+}
+
+/**
  * @brief Deallocate log format
  * @details Deallocates the entire log format structure
  * @param[in] p_format The structure to deallocate