From: Michal Bloch Date: Wed, 26 Apr 2023 16:08:10 +0000 (+0200) Subject: Extract common parts of util req handling X-Git-Tag: accepted/tizen/unified/20230818.054520~5 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ce49a70191da62f3688fa98e0094ad946ff026f8;p=platform%2Fcore%2Fsystem%2Fdlog.git Extract common parts of util req handling Change-Id: I8cec8a8c8f20724714b149354baea1fb3f94df16 Signed-off-by: Michal Bloch --- diff --git a/src/logger/log_buffer.c b/src/logger/log_buffer.c index ed5fdbb..9ebc7fe 100644 --- 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(¶ms, (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, ¶ms); - 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(¶ms, (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, ¶ms); - 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(¶ms, 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, ¶ms); + if (r != 0) goto cleanup; - retval = add_reader_to_server(&reader->common, server); - if (retval < 0) + if (!distribute_pipes(server, reader, ¶ms, 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)