From ec578943264e0161944507ba004dba532deb08ce Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Tue, 28 Jul 2020 22:49:03 +0200 Subject: [PATCH] daemon: split util commandline parsing Change-Id: Id8154e0090b1be067e2b2799d110fb4750ab531a --- src/logger/logger.c | 249 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 148 insertions(+), 101 deletions(-) diff --git a/src/logger/logger.c b/src/logger/logger.c index bbb18fc..123fe8e 100644 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -1253,59 +1253,69 @@ static void reset_getopt_internals(void *fake) optopt = 0; } -/** - * @brief Parse a command line - * @details Creates a reader from a parsed command line - * @param[in] cmdl The command line string - * @param[in] wr The writer who sent the line (NULL if internal) - * @param[in] server The server - * @param[out] rd The new reader - * @return 0 on success, else -errno - */ -static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, struct logger *server, struct reader_any *rd) +struct dlogutil_line_params { + bool monitor; + bool is_dumping; + struct log_file file; + log_id_t buf_id; + char *file_path; + struct dlogutil_filter_options *filter; +}; + +static bool initialize_dlogutil_line_params(struct dlogutil_line_params *params) { - assert(cmdl); - // wr optional: NULL means created by config - assert(server); - assert(rd); + assert(params); - char cmdline[512]; - int argc = 0; - char *argv[100]; - char *tok; - char *tok_sv; - int retval = 0; - int silence = 0; + logfile_init(¶ms->file); + params->monitor = false; + params->is_dumping = false; + params->buf_id = LOG_ID_INVALID; + params->file_path = NULL; - snprintf(cmdline, sizeof(cmdline), "%s", cmdl); + params->filter = log_filter_new(); + if (!params->filter) + return false; - bool reader_init = false; - struct reader_any reader = {}; + return true; +} - struct log_file file; - logfile_init(&file); +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); + assert(argc); + assert(argv); - bool is_dumping = false; - bool monitor = false; - log_id_t buf_id = LOG_ID_INVALID; + snprintf(buffer, buf_size, "%s", cmdl); // strtok is destructive - struct dlogutil_filter_options *filter = log_filter_new(); - if (!filter) { - retval = -ENOMEM; - goto cleanup; - } + char *tok_sv; + char *tok = strtok_r(buffer, DELIMITER, &tok_sv); - tok = strtok_r(cmdline, DELIMITER, &tok_sv); - if (!tok || strcmp(tok, "dlogutil")) { - retval = -EINVAL; - goto cleanup; - } + /* Legacy requirement: emulate `dlogutil` + * invocations right up to the binary name */ + if (!tok || strcmp(tok, "dlogutil")) + return -EINVAL; - while (tok && (argc < NELEMS(argv))) { - argv[argc++] = tok; + int curr_argc = 0; + const int argv_size = *argc - 1; // for NULL at the end + while (tok && (curr_argc < argv_size)) { + argv[curr_argc++] = tok; tok = strtok_r(NULL, DELIMITER, &tok_sv); } + argv[curr_argc] = NULL; + *argc = curr_argc; + + return 0; +} + +static int get_dlogutil_params_from_argc_argv(int argc, char **argv, struct dlogutil_line_params *params) +{ + assert(argc >= 0); + assert(argv); + assert(params); + + bool silence = false; + static const struct option long_options[] = { {"tid", required_argument, NULL, 0}, {"pid", required_argument, NULL, 1}, @@ -1318,77 +1328,51 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, break; case 'd': case 't': - is_dumping = true; + params->is_dumping = true; break; case 'm': - monitor = true; + params->monitor = true; break; case 'f': - if (wr) { - /* 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; - goto cleanup; - } - - retval = logfile_set_path(&file, optarg); - if (retval < 0) - goto cleanup; + free(params->file_path); + params->file_path = strdup(optarg); + if (!params->file_path) + return -ENOMEM; break; case 'b': - assert(g_backend.reader_any_init); - - buf_id = log_id_by_name(optarg); - if (buf_id == LOG_ID_INVALID) { - retval = -EINVAL; - goto cleanup; - } + params->buf_id = log_id_by_name(optarg); + if (params->buf_id == LOG_ID_INVALID) + return -EINVAL; break; case 'r': - if (sscanf(optarg, "%zu", &file.rotate_size_kbytes) != 1) { - retval = -EINVAL; - goto cleanup; - } + if (sscanf(optarg, "%zu", ¶ms->file.rotate_size_kbytes) != 1) + return -EINVAL; break; case 'n': - if (sscanf(optarg, "%zu", &file.max_rotated) != 1) { - retval = -EINVAL; - goto cleanup; - } + if (sscanf(optarg, "%zu", ¶ms->file.max_rotated) != 1) + return -EINVAL; break; case 'v': - file.format.format = log_format_from_string(optarg); + params->file.format.format = log_format_from_string(optarg); break; case 's': - silence = 1; - dlogutil_filter_options_set_filterspec(filter, "*:s"); + silence = true; + dlogutil_filter_options_set_filterspec(params->filter, "*:s"); break; case 0: { pid_t tid; - if (sscanf(optarg, "%d", &tid) != 1 || dlogutil_filter_options_set_tid(filter, tid)) { - retval = -EINVAL; - goto cleanup; - } + if (sscanf(optarg, "%d", &tid) != 1 || dlogutil_filter_options_set_tid(params->filter, tid)) + return -EINVAL; break; } case 1: { pid_t pid; - if (sscanf(optarg, "%d", &pid) != 1 || dlogutil_filter_options_set_pid(filter, pid)) { - retval = -EINVAL; - goto cleanup; - } + if (sscanf(optarg, "%d", &pid) != 1 || dlogutil_filter_options_set_pid(params->filter, pid)) + return -EINVAL; break; } case '?': // invalid option or missing argument - retval = -EINVAL; - goto cleanup; + return -EINVAL; default: // everything else gets handled in util directly break; @@ -1396,31 +1380,93 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, } // dump + monitor = continuous - if (monitor && is_dumping) { - is_dumping = false; - monitor = false; + if (params->monitor && params->is_dumping) { + params->is_dumping = false; + params->monitor = false; } if (optind < argc) while (optind < argc) - dlogutil_filter_options_set_filterspec(filter, argv[optind++]); + dlogutil_filter_options_set_filterspec(params->filter, argv[optind++]); else if (!silence) - dlogutil_filter_options_set_filterspec(filter, "*:D"); + dlogutil_filter_options_set_filterspec(params->filter, "*:D"); + + return 0; +} + +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 + int argc = NELEMS(argv); + int ret = make_argc_argv_from_dlogutil_line(cmdl, sizeof buffer, buffer, &argc, argv); + if (ret < 0) + return ret; + + return get_dlogutil_params_from_argc_argv(argc, argv, params); +} + +/** + * @brief Parse a command line + * @details Creates a reader from a parsed command line + * @param[in] cmdl The command line string + * @param[in] wr The writer who sent the line (NULL if internal) + * @param[in] server The server + * @param[out] rd The new reader + * @return 0 on success, else -errno + */ +static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, struct logger *server, struct reader_any *rd) +{ + assert(cmdl); + // wr optional: NULL means created by config + assert(server); + assert(rd); + + bool reader_init = false; + struct reader_any reader = {}; + + struct dlogutil_line_params params; + int retval = get_dlogutil_line_params(cmdl, ¶ms); + if (retval < 0) + goto cleanup; + + if (params.file_path) { + if (wr) { + /* 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; + goto cleanup; + } + + retval = logfile_set_path(¶ms.file, params.file_path); + if (retval < 0) + goto cleanup; - if (file.path != NULL) { - retval = logfile_open(&file); + retval = logfile_open(¶ms.file); if (retval < 0) goto cleanup; } if (wr && wr->buf_ptr) { struct reader_pipe *pipe = NULL; - retval = reader_pipe_init_with_writer(&pipe, wr, server, filter, file, monitor, is_dumping); + retval = reader_pipe_init_with_writer(&pipe, wr, server, params.filter, params.file, params.monitor, params.is_dumping); reader.which = READER_ANY_PIPE; reader.pipe_inner = pipe; } - else if (buf_id != LOG_ID_INVALID) - retval = g_backend.reader_any_init(&reader, buf_id, server, filter, file, monitor, is_dumping); + else if (params.buf_id != LOG_ID_INVALID) + retval = g_backend.reader_any_init(&reader, params.buf_id, server, params.filter, params.file, params.monitor, params.is_dumping); else retval = -EINVAL; if (retval != 0) @@ -1432,7 +1478,7 @@ static int create_reader_from_dlogutil_line(const char *cmdl, struct writer *wr, goto cleanup; } - if (file.path == NULL) { + if (params.file.path == NULL) { if (wr) { assert(reader.which == READER_ANY_PIPE); int write_fd = -1, read_fd = -1; @@ -1461,9 +1507,10 @@ cleanup: if (reader_init) reader_any_free(reader); else { - logfile_free(&file); - log_filter_free(filter); + logfile_free(¶ms.file); + log_filter_free(params.filter); } + free(params.file_path); } else *rd = reader; return retval; -- 2.7.4