Extract common parts of util req handling

Change-Id: I8cec8a8c8f20724714b149354baea1fb3f94df16
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/logger/log_buffer.c b/src/logger/log_buffer.c
index ed5fdbb..9ebc7fe 100644 (file)
--- a/src/logger/log_buffer.c
+++ b/src/logger/log_buffer.c
@@ -195,189 +195,126 @@ static int service_writer_stdout(struct logger *server, struct writer *wr, struc
        return ret;
 }
 
-/**
- * @brief Service util request
- * @details Handle a standard (not compressed memory) request from util
- * @param[in] server The logger server
- * @param[in] wr The writer who sent the request
- * @param[in] msg The message containing the request
- * @return 0 on success, else -errno
- */
-static int service_writer_handle_req_util(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+static bool is_control_msg_valid_for_util_req(const struct dlog_control_msg *msg)
 {
-       assert(server);
-       assert(wr);
-       assert(msg);
-
-       assert(msg->request == DLOG_REQ_HANDLE_LOGUTIL);
-
-       if (msg->length <= sizeof(struct dlog_control_msg) ||
-           msg->length > sizeof(struct dlog_control_msg) + MAX_LOGGER_REQUEST_LEN)
-               return -EINVAL;
-
-       if (msg->data[msg->length - sizeof(struct dlog_control_msg)] != 0)
-               return -EINVAL;
-
-       __attribute__((cleanup(reader_free_ptr))) struct reader_pipe *reader = NULL;
+       return msg->length >  sizeof *msg
+           && msg->length <= sizeof *msg + MAX_LOGGER_REQUEST_LEN
+           && msg->data[msg->length - sizeof *msg] == '\0'
+       ;
+}
 
-       int retval;
-       __attribute__((cleanup(free_dlogutil_line_params))) struct dlogutil_line_params params;
-       if (!initialize_dlogutil_line_params(&params, (struct buf_params) { })) {
-               /* TODO: cleanup discards this value, so there isn't much
-                * point setting it. Ideally it would be attached to the
-                * reply but that's a protocol change so not worth it atm */
-               // retval = -ENOMEM;
-               goto cleanup;
-       }
+static int parse_req_for_dlogutil_params(struct dlogutil_line_params *params, const struct dlog_control_msg *msg)
+{
+       if (!initialize_dlogutil_line_params(params, (struct buf_params) { }))
+               return -ENOMEM;
 
-       /* Note that in this case, the format doesn't matter.
+       /* Note that in this case, the format doesn't matter
+        * since we're going to send binary data anyway.
         * Therefore, we just pass OFF as default. */
-       retval = get_dlogutil_line_params(msg->data, FORMAT_OFF, &params);
-       if (retval < 0) {
-               // retval = -ENOMEM; // see above
-               goto cleanup;
-       }
+       const int r = get_dlogutil_line_params(msg->data, FORMAT_OFF, params);
+       if (r < 0)
+               return r;
 
-       if (params.compression) {
-               /* Memory compression is only available from the text
-                * config. libdlogutil clients have a separate request
-                * type to obtain data. Eventually it would be good to
-                * merge them together, but that requires some thought
-                * put into it. */
-               goto cleanup;
-       }
+       /* Do not trust writer-based readers (only config-based).
+        * The control socket's privilege checks are fairly lenient
+        * so this prevents people from asking us to overwrite
+        * some potentially important files at logger privilege.
+        *
+        * At some point it would be good to be able to skip the
+        * middleman and become able to write to a file directly
+        * though. The daemon should become able to receive an
+        * opened file descriptor from a writer. */
+       if (params->file_path)
+               return -EPERM;
 
-       if (params.file_path) {
-               /* Do not trust writer-based readers (only config-based).
-                * The control socket's privilege checks are fairly lenient
-                * so this prevents people from asking us to overwrite
-                * some potentially important files at logger privilege.
-                *
-                * At some point it would be good to be able to skip the
-                * middleman and become able to write to a file directly
-                * though. The daemon should become able to receive an
-                * opened file descriptor from a writer. */
-               // retval = -EPERM; // see above
-               goto cleanup;
-       }
+       return 0;
+}
 
-       retval = reader_pipe_init(&reader, params.enabled_buffers, server, params.monitor, params.is_dumping);
-       if (retval != 0)
-               goto cleanup;
+static int sent_req_reply(int fd, signed char request)
+{
+       const int ret = send_dlog_reply(fd, request, DLOG_REQ_RESULT_ERR, NULL, 0);
+       if (ret < 0)
+               printf("ERROR: both the request handling and send_dlog_reply() failed\n");
+
+       return ret;
+}
 
+static int distribute_pipes(struct logger *server, struct reader_common *reader, struct dlogutil_line_params *params, int writer_fd)
+{
        int pipe_fd[2] = { -1, -1 };
-       retval = create_fifo_fds(server, pipe_fd, 0, params.is_dumping);
-       if (retval < 0)
-               goto cleanup;
+       if (create_fifo_fds(server, pipe_fd, 0, params->is_dumping))
+               return false;
 
-       retval = reader_add_subreader_dlogutil(&reader->common, params.filter, pipe_fd[1]);
-       if (retval != 0)
-               goto cleanup;
+       if (reader_add_subreader_dlogutil(reader, params->filter, pipe_fd[1]) != 0)
+               return false;
 
-       set_write_fd_entity(&reader->common.fd_entity_sink, pipe_fd[1]);
-       retval = send_pipe(wr->fd_entity.fd, pipe_fd[0]);
+       /* FIXME: ideally the FD entity would belong to the sub,
+        * it only works because there is only one sub at a time.
+        * Changing this requires some design thought around how
+        * to handle multiple subs with varying sink throughput. */
+       set_write_fd_entity(&reader->fd_entity_sink, pipe_fd[1]);
+
+       const int rs = send_pipe(writer_fd, pipe_fd[0]);
        if (pipe_fd[0] > 0)
                close(pipe_fd[0]);
-       if (retval)
-               goto cleanup;
+       if (rs)
+               return false;
 
-       retval = add_reader_to_server(&reader->common, server);
-       if (retval < 0)
-               goto cleanup;
+       const int ra = add_reader_to_server(reader, server);
+       if (ra < 0)
+               return false;
 
-       reader = NULL;
-       return 0;
+       return true;
+}
 
-cleanup:
-       /* NB: reply success means that stuff is still stable enough that the client
-        * can probably get a second chance, so return a success from the whole func
-        * so as not to drop the client */
-       retval = send_dlog_reply(wr->fd_entity.fd, DLOG_REQ_HANDLE_LOGUTIL, DLOG_REQ_RESULT_ERR, NULL, 0);
-       if (retval < 0)
-               printf("ERROR: both create_reader_from_dlogutil_line() and send_dlog_reply() failed\n");
+int req_init_reader_pipe(struct logger *server, struct writer *wr, void *reader, struct dlogutil_line_params *params)
+{
+       return reader_pipe_init((struct reader_pipe **)reader, params->enabled_buffers, server, params->monitor, params->is_dumping);
+}
 
-       return retval;
+int req_init_reader_memory(struct logger *server, struct writer *wr, void *reader, struct dlogutil_line_params *params)
+{
+       return reader_memory_init_with_writer((struct reader_memory **)reader, wr, server, params->compression, params->monitor, params->is_dumping);
 }
 
-static int service_writer_handle_req_compressed_memory_util(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+/**
+ * @brief Service util request
+ * @details Handle a request from util
+ * @param[in] server The logger server
+ * @param[in] wr The writer who sent the request
+ * @param[in] msg The message containing the request
+ * @return 0 on success, else -errno
+ */
+int service_writer_handle_req_generic_util(struct logger *server, struct writer *wr, struct dlog_control_msg *msg, bool wanted_compression,
+       int (*make_reader_func)(struct logger *, struct writer *, void *, struct dlogutil_line_params *), signed char request)
 {
        assert(server);
        assert(wr);
        assert(msg);
+       assert(msg->request == request);
 
-       assert(msg->request == DLOG_REQ_HANDLE_COMPRESSED_LOGUTIL);
-
-       if (msg->length <= sizeof(struct dlog_control_msg) ||
-           msg->length > sizeof(struct dlog_control_msg) + MAX_LOGGER_REQUEST_LEN)
+       if (!is_control_msg_valid_for_util_req(msg))
                return -EINVAL;
 
-       if (msg->data[msg->length - sizeof(struct dlog_control_msg)] != 0)
-               return -EINVAL;
-
-       __attribute__((cleanup(reader_free_ptr))) struct reader_memory *reader = NULL;
-
-       int retval;
        __attribute__((cleanup(free_dlogutil_line_params))) struct dlogutil_line_params params;
-       if (!initialize_dlogutil_line_params(&params, (struct buf_params) { })) {
-               /* TODO: cleanup discards this value, so there isn't much
-                * point setting it. Ideally it would be attached to the
-                * reply but that's a protocol change so not worth it atm */
-               // retval = -ENOMEM;
-               goto cleanup;
-       }
-
-       /* Note that in this case, the format doesn't matter.
-        * Therefore, we just pass OFF as default. */
-       retval = get_dlogutil_line_params(msg->data, FORMAT_OFF, &params);
-       if (retval < 0) {
-               // retval = -ENOMEM; // see above
-               goto cleanup;
-       }
-
-       if (params.file_path) {
-               /* Do not trust writer-based readers (only config-based).
-                * The control socket's privilege checks are fairly lenient
-                * so this prevents people from asking us to overwrite
-                * some potentially important files at logger privilege.
-                *
-                * At some point it would be good to be able to skip the
-                * middleman and become able to write to a file directly
-                * though. The daemon should become able to receive an
-                * opened file descriptor from a writer. */
-               // retval = -EPERM; // see above
-               goto cleanup;
-       }
-
-       if (!params.compression)
-               goto cleanup;
-
-       retval = reader_memory_init_with_writer(&reader, wr, server, params.compression, params.monitor, params.is_dumping);
-       if (retval != 0)
-               goto cleanup;
-
-       int pipe_fd[2] = { -1, -1 };
-       retval = create_fifo_fds(server, pipe_fd, 0, reader->is_dumping);
-       if (retval < 0)
+       int r = parse_req_for_dlogutil_params(&params, msg);
+       if (r < 0)
                goto cleanup;
 
-       retval = reader_add_subreader_dlogutil(&reader->common, params.filter, pipe_fd[1]);
-       if (retval != 0)
+       /* Enabling compression is only available from the text
+        * config. libdlogutil clients have a separate request
+        * type to obtain data. Eventually it would be good to
+        * merge them together, but that requires some thought
+        * put into it. */
+       if (!!params.compression != wanted_compression)
                goto cleanup;
 
-       /* FIXME: ideally the FD entity would belong to the sub,
-        * it only works because there is only one sub at a time.
-        * Changing this requires some design thought around how
-        * to handle multiple subs with varying sink throughput. */
-       set_write_fd_entity(&reader->common.fd_entity_sink, pipe_fd[1]);
-
-       retval = send_pipe(wr->fd_entity.fd, pipe_fd[0]);
-       if (pipe_fd[0] > 0)
-               close(pipe_fd[0]);
-       if (retval)
+       __attribute__((cleanup(reader_free_ptr))) struct reader_common *reader = NULL;
+       r = make_reader_func(server, wr, &reader, &params);
+       if (r != 0)
                goto cleanup;
 
-       retval = add_reader_to_server(&reader->common, server);
-       if (retval < 0)
+       if (!distribute_pipes(server, reader, &params, wr->fd_entity.fd))
                goto cleanup;
 
        reader = NULL;
@@ -387,11 +324,19 @@ cleanup:
        /* NB: reply success means that stuff is still stable enough that the client
         * can probably get a second chance, so return a success from the whole func
         * so as not to drop the client */
-       retval = send_dlog_reply(wr->fd_entity.fd, DLOG_REQ_HANDLE_COMPRESSED_LOGUTIL, DLOG_REQ_RESULT_ERR, NULL, 0);
-       if (retval < 0)
-               printf("ERROR: both create_reader_from_dlogutil_line() and send_dlog_reply() failed\n");
+       return sent_req_reply(wr->fd_entity.fd, request);
+}
 
-       return retval;
+static int service_writer_handle_req_util(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+{
+       return service_writer_handle_req_generic_util(server, wr, msg,
+               false, req_init_reader_pipe, DLOG_REQ_HANDLE_LOGUTIL);
+}
+
+static int service_writer_handle_req_compressed_memory_util(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+{
+       return service_writer_handle_req_generic_util(server, wr, msg,
+               true, req_init_reader_memory, DLOG_REQ_HANDLE_COMPRESSED_LOGUTIL);
 }
 
 static int service_writer_handle_req_get_usage(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)