daemon: split util commandline parsing 68/239668/3
authorMichal Bloch <m.bloch@samsung.com>
Tue, 28 Jul 2020 20:49:03 +0000 (22:49 +0200)
committerMichal Bloch <m.bloch@samsung.com>
Tue, 4 Aug 2020 15:59:39 +0000 (17:59 +0200)
Change-Id: Id8154e0090b1be067e2b2799d110fb4750ab531a

src/logger/logger.c

index bbb18fc..123fe8e 100644 (file)
@@ -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(&params->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", &params->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", &params->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, &params);
+       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(&params.file, params.file_path);
+               if (retval < 0)
+                       goto cleanup;
 
-       if (file.path != NULL) {
-               retval = logfile_open(&file);
+               retval = logfile_open(&params.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(&params.file);
+                       log_filter_free(params.filter);
                }
+               free(params.file_path);
        } else
                *rd = reader;
        return retval;