From 326d2ef234cd8838700e914a0d780f46be50904c Mon Sep 17 00:00:00 2001 From: nicm Date: Sat, 21 Aug 2021 18:39:07 +0000 Subject: [PATCH 1/3] Pass typed arguments out of the parser into the arguments list and let it convert them into strings. --- arguments.c | 125 +++++++++++++++++++++++++++++++++++++++++++--------- cmd-parse.y | 29 +++++++----- cmd.c | 25 ++++------- tmux.h | 20 +++++++-- 4 files changed, 148 insertions(+), 51 deletions(-) diff --git a/arguments.c b/arguments.c index 2f9c0c31..4fc83a0a 100644 --- a/arguments.c +++ b/arguments.c @@ -18,9 +18,9 @@ #include +#include #include #include -#include #include #include "tmux.h" @@ -66,6 +66,20 @@ args_find(struct args *args, u_char flag) return (RB_FIND(args_tree, &args->tree, &entry)); } +/* Get value as string. */ +static char * +args_value_as_string(struct args_value *value) +{ + switch (value->type) { + case ARGS_NONE: + return (xstrdup("")); + case ARGS_COMMANDS: + return (cmd_list_print(value->cmdlist, 0)); + case ARGS_STRING: + return (xstrdup(value->string)); + } +} + /* Create an empty arguments set. */ struct args * args_create(void) @@ -77,42 +91,109 @@ args_create(void) return (args); } -/* Parse an argv and argc into a new argument set. */ +/* Parse arguments into a new argument set. */ struct args * -args_parse(const struct args_parse *parse, int argc, char **argv) +args_parse(const struct args_parse *parse, struct args_value *values, + u_int count) { - struct args *args; - int opt; + struct args *args; + u_int i; + struct args_value *value; + u_char flag, argument; + const char *found, *string; + char *s; - optreset = 1; - optind = 1; - optarg = NULL; + if (count == 0) + return (args_create()); args = args_create(); - while ((opt = getopt(argc, argv, parse->template)) != -1) { - if (opt < 0) - continue; - if (opt == '?' || strchr(parse->template, opt) == NULL) { - args_free(args); - return (NULL); + for (i = 1; i < count; /* nothing */) { + value = &values[i]; + + s = args_value_as_string(value); + log_debug("%s: %u = %s", __func__, i, s); + free(s); + + if (value->type != ARGS_STRING) + break; + + string = value->string; + if (*string++ != '-' || *string == '\0') + break; + i++; + if (string[0] == '-' && string[1] == '\0') + break; + + for (;;) { + flag = *string++; + if (flag == '\0') + break; + if (!isalnum(flag)) { + args_free(args); + return (NULL); + } + found = strchr(parse->template, flag); + if (found == NULL) { + args_free(args); + return (NULL); + } + argument = *++found; + if (argument != ':') { + log_debug("%s: add -%c", __func__, flag); + args_set(args, flag, NULL); + continue; + } + if (*string != '\0') + s = xstrdup(string); + else { + if (i == count) { + args_free(args); + return (NULL); + } + s = args_value_as_string(&values[i++]); + } + log_debug("%s: add -%c = %s", __func__, flag, s); + args_set(args, flag, s); + free(s); + break; } - args_set(args, opt, optarg); - optarg = NULL; } - argc -= optind; - argv += optind; + log_debug("%s: flags end at %u of %u", __func__, i, count); + if (i != count) { + for (/* nothing */; i < count; i++) { + value = &values[i]; - args->argc = argc; - args->argv = cmd_copy_argv(argc, argv); + s = args_value_as_string(value); + log_debug("%s: %u = %s", __func__, i, s); + cmd_append_argv(&args->argc, &args->argv, s); + free(s); + } + } - if ((parse->lower != -1 && argc < parse->lower) || - (parse->upper != -1 && argc > parse->upper)) { + if ((parse->lower != -1 && args->argc < parse->lower) || + (parse->upper != -1 && args->argc > parse->upper)) { args_free(args); return (NULL); } return (args); } +/* Free a value. */ +void +args_free_value(struct args_value *value) +{ + switch (value->type) { + case ARGS_NONE: + break; + case ARGS_STRING: + free(value->string); + break; + case ARGS_COMMANDS: + cmd_list_free(value->cmdlist); + break; + } +} + /* Free an arguments set. */ void args_free(struct args *args) diff --git a/cmd-parse.y b/cmd-parse.y index 6be5d8a0..57d0e84e 100644 --- a/cmd-parse.y +++ b/cmd-parse.y @@ -794,39 +794,48 @@ cmd_parse_build_command(struct cmd_parse_command *cmd, struct cmd_parse_input *pi, struct cmd_parse_result *pr) { struct cmd_parse_argument *arg; - struct cmd_list *cmdlist; + struct cmd_list *cmdlist = NULL; struct cmd *add; - char *s, **argv = NULL, *cause; - int argc = 0; + char *cause; + struct args_value *values = NULL; + u_int count = 0, idx; if (cmd_parse_expand_alias(cmd, pi, pr, &cmdlist)) return (cmdlist); TAILQ_FOREACH(arg, &cmd->arguments, entry) { + values = xreallocarray(values, count + 1, sizeof *values); switch (arg->type) { case CMD_PARSE_STRING: - cmd_append_argv(&argc, &argv, arg->string); + values[count].type = ARGS_STRING; + values[count].string = xstrdup(arg->string); break; case CMD_PARSE_COMMANDS: cmd_parse_build_commands(arg->commands, pi, pr); if (pr->status != CMD_PARSE_SUCCESS) - return (NULL); - s = cmd_list_print(pr->cmdlist, 0); - cmd_append_argv(&argc, &argv, s); - free(s); + goto out; + values[count].type = ARGS_COMMANDS; + values[count].cmdlist = pr->cmdlist; + values[count].cmdlist->references++; break; } + count++; } - add = cmd_parse(argc, argv, pi->file, pi->line, &cause); + add = cmd_parse(values, count, pi->file, pi->line, &cause); if (add == NULL) { pr->status = CMD_PARSE_ERROR; pr->error = cmd_parse_get_error(pi->file, pi->line, cause); free(cause); - return (NULL); + goto out; } cmdlist = cmd_list_new(); cmd_list_append(cmdlist, add); + +out: + for (idx = 0; idx < count; idx++) + args_free_value(&values[idx]); + free(values); return (cmdlist); } diff --git a/cmd.c b/cmd.c index ce04ce2a..930ee56c 100644 --- a/cmd.c +++ b/cmd.c @@ -496,27 +496,26 @@ ambiguous: /* Parse a single command from an argument vector. */ struct cmd * -cmd_parse(int argc, char **argv, const char *file, u_int line, char **cause) +cmd_parse(struct args_value *values, u_int count, const char *file, u_int line, + char **cause) { const struct cmd_entry *entry; - const char *name; struct cmd *cmd; struct args *args; - if (argc == 0) { + if (count == 0 || values[0].type != ARGS_STRING) { xasprintf(cause, "no command"); return (NULL); } - name = argv[0]; - - entry = cmd_find(name, cause); + entry = cmd_find(values[0].string, cause); if (entry == NULL) return (NULL); - cmd_log_argv(argc, argv, "%s: %s", __func__, entry->name); - args = args_parse(&entry->args, argc, argv); - if (args == NULL) - goto usage; + args = args_parse(&entry->args, values, count); + if (args == NULL) { + xasprintf(cause, "usage: %s %s", entry->name, entry->usage); + return (NULL); + } cmd = xcalloc(1, sizeof *cmd); cmd->entry = entry; @@ -527,12 +526,6 @@ cmd_parse(int argc, char **argv, const char *file, u_int line, char **cause) cmd->line = line; return (cmd); - -usage: - if (args != NULL) - args_free(args); - xasprintf(cause, "usage: %s %s", entry->name, entry->usage); - return (NULL); } /* Free a command. */ diff --git a/tmux.h b/tmux.h index f8dcac49..34c66493 100644 --- a/tmux.h +++ b/tmux.h @@ -1355,9 +1355,20 @@ struct message_entry { }; TAILQ_HEAD(message_list, message_entry); +/* Argument type. */ +enum args_type { + ARGS_NONE, + ARGS_STRING, + ARGS_COMMANDS +}; + /* Argument value. */ struct args_value { - char *string; + enum args_type type; + union { + char *string; + struct cmd_list *cmdlist; + }; TAILQ_ENTRY(args_value) entry; }; @@ -2187,8 +2198,10 @@ int tty_keys_next(struct tty *); /* arguments.c */ void args_set(struct args *, u_char, const char *); struct args *args_create(void); -struct args *args_parse(const struct args_parse *, int, char **); +struct args *args_parse(const struct args_parse *, struct args_value *, + u_int); void args_vector(struct args *, int *, char ***); +void args_free_value(struct args_value *); void args_free(struct args *); char *args_print(struct args *); char *args_escape(const char *); @@ -2250,7 +2263,8 @@ const struct cmd_entry *cmd_get_entry(struct cmd *); struct args *cmd_get_args(struct cmd *); u_int cmd_get_group(struct cmd *); void cmd_get_source(struct cmd *, const char **, u_int *); -struct cmd *cmd_parse(int, char **, const char *, u_int, char **); +struct cmd *cmd_parse(struct args_value *, u_int, const char *, u_int, + char **); void cmd_free(struct cmd *); char *cmd_print(struct cmd *); struct cmd_list *cmd_list_new(void); From 069f5925af8bf70a99eec7f4baf5772707e62def Mon Sep 17 00:00:00 2001 From: nicm Date: Sat, 21 Aug 2021 20:46:43 +0000 Subject: [PATCH 2/3] Preserve argument type in command and convert to string on demand. --- cmd-bind-key.c | 45 +++++++++++++++++++++++++++------------------ cmd-find-window.c | 34 +++++++++++++++++++++------------- cmd-parse.y | 3 ++- tmux.h | 4 +++- 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/cmd-bind-key.c b/cmd-bind-key.c index 97057b3a..4a6c8541 100644 --- a/cmd-bind-key.c +++ b/cmd-bind-key.c @@ -50,6 +50,7 @@ cmd_bind_key_exec(struct cmd *self, struct cmdq_item *item) struct cmd_parse_result *pr; char **argv; int argc, repeat; + struct args_value *value; u_int count = args_count(args); key = key_string_lookup_string(args_string(args, 0)); @@ -66,24 +67,32 @@ cmd_bind_key_exec(struct cmd *self, struct cmdq_item *item) tablename = "prefix"; repeat = args_has(args, 'r'); - if (count != 1) { - if (count == 2) - pr = cmd_parse_from_string(args_string(args, 1), NULL); - else { - args_vector(args, &argc, &argv); - pr = cmd_parse_from_arguments(argc - 1, argv + 1, NULL); - cmd_free_argv(argc, argv); - } - switch (pr->status) { - case CMD_PARSE_ERROR: - cmdq_error(item, "%s", pr->error); - free(pr->error); - return (CMD_RETURN_ERROR); - case CMD_PARSE_SUCCESS: - break; - } - key_bindings_add(tablename, key, note, repeat, pr->cmdlist); - } else + if (count == 1) { key_bindings_add(tablename, key, note, repeat, NULL); + return (CMD_RETURN_NORMAL); + } + + value = args_value(args, 1); + if (count == 2 && value->type == ARGS_COMMANDS) { + key_bindings_add(tablename, key, note, repeat, value->cmdlist); + return (CMD_RETURN_NORMAL); + } + + if (count == 2) + pr = cmd_parse_from_string(args_string(args, 1), NULL); + else { + args_vector(args, &argc, &argv); + pr = cmd_parse_from_arguments(argc - 1, argv + 1, NULL); + cmd_free_argv(argc, argv); + } + switch (pr->status) { + case CMD_PARSE_ERROR: + cmdq_error(item, "%s", pr->error); + free(pr->error); + return (CMD_RETURN_ERROR); + case CMD_PARSE_SUCCESS: + break; + } + key_bindings_add(tablename, key, note, repeat, pr->cmdlist); return (CMD_RETURN_NORMAL); } diff --git a/cmd-find-window.c b/cmd-find-window.c index 804e8fe4..6e07537c 100644 --- a/cmd-find-window.c +++ b/cmd-find-window.c @@ -48,7 +48,7 @@ cmd_find_window_exec(struct cmd *self, struct cmdq_item *item) struct cmd_find_state *target = cmdq_get_target(item); struct window_pane *wp = target->wp; const char *s = args_string(args, 0), *suffix = ""; - char *filter; + struct args_value *filter; int C, N, T; C = args_has(args, 'C'); @@ -65,31 +65,41 @@ cmd_find_window_exec(struct cmd *self, struct cmdq_item *item) if (!C && !N && !T) C = N = T = 1; + filter = xcalloc(1, sizeof *filter); + filter->type = ARGS_STRING; + if (C && N && T) { - xasprintf(&filter, + xasprintf(&filter->string, "#{||:" "#{C%s:%s},#{||:#{m%s:*%s*,#{window_name}}," "#{m%s:*%s*,#{pane_title}}}}", suffix, s, suffix, s, suffix, s); } else if (C && N) { - xasprintf(&filter, + xasprintf(&filter->string, "#{||:#{C%s:%s},#{m%s:*%s*,#{window_name}}}", suffix, s, suffix, s); } else if (C && T) { - xasprintf(&filter, + xasprintf(&filter->string, "#{||:#{C%s:%s},#{m%s:*%s*,#{pane_title}}}", suffix, s, suffix, s); } else if (N && T) { - xasprintf(&filter, + xasprintf(&filter->string, "#{||:#{m%s:*%s*,#{window_name}}," "#{m%s:*%s*,#{pane_title}}}", suffix, s, suffix, s); - } else if (C) - xasprintf(&filter, "#{C%s:%s}", suffix, s); - else if (N) - xasprintf(&filter, "#{m%s:*%s*,#{window_name}}", suffix, s); - else - xasprintf(&filter, "#{m%s:*%s*,#{pane_title}}", suffix, s); + } else if (C) { + xasprintf(&filter->string, + "#{C%s:%s}", + suffix, s); + } else if (N) { + xasprintf(&filter->string, + "#{m%s:*%s*,#{window_name}}", + suffix, s); + } else { + xasprintf(&filter->string, + "#{m%s:*%s*,#{pane_title}}", + suffix, s); + } new_args = args_create(); if (args_has(args, 'Z')) @@ -97,9 +107,7 @@ cmd_find_window_exec(struct cmd *self, struct cmdq_item *item) args_set(new_args, 'f', filter); window_pane_set_mode(wp, NULL, &window_tree_mode, target, new_args); - args_free(new_args); - free(filter); return (CMD_RETURN_NORMAL); } diff --git a/cmd-parse.y b/cmd-parse.y index 57d0e84e..86917941 100644 --- a/cmd-parse.y +++ b/cmd-parse.y @@ -804,7 +804,8 @@ cmd_parse_build_command(struct cmd_parse_command *cmd, return (cmdlist); TAILQ_FOREACH(arg, &cmd->arguments, entry) { - values = xreallocarray(values, count + 1, sizeof *values); + values = xrecallocarray(values, count, count + 1, + sizeof *values); switch (arg->type) { case CMD_PARSE_STRING: values[count].type = ARGS_STRING; diff --git a/tmux.h b/tmux.h index 34c66493..fa0268a2 100644 --- a/tmux.h +++ b/tmux.h @@ -1369,6 +1369,7 @@ struct args_value { char *string; struct cmd_list *cmdlist; }; + char *cached; TAILQ_ENTRY(args_value) entry; }; @@ -2196,7 +2197,7 @@ void tty_keys_free(struct tty *); int tty_keys_next(struct tty *); /* arguments.c */ -void args_set(struct args *, u_char, const char *); +void args_set(struct args *, u_char, struct args_value *); struct args *args_create(void); struct args *args_parse(const struct args_parse *, struct args_value *, u_int); @@ -2210,6 +2211,7 @@ const char *args_get(struct args *, u_char); u_char args_first(struct args *, struct args_entry **); u_char args_next(struct args_entry **); u_int args_count(struct args *); +struct args_value *args_value(struct args *, u_int); const char *args_string(struct args *, u_int); struct args_value *args_first_value(struct args *, u_char); struct args_value *args_next_value(struct args_value *); From 0084cbef5ad9f1f860b50fb7c485ac841817b3a8 Mon Sep 17 00:00:00 2001 From: nicm Date: Sat, 21 Aug 2021 20:57:52 +0000 Subject: [PATCH 3/3] Free value properly. --- arguments.c | 143 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 93 insertions(+), 50 deletions(-) diff --git a/arguments.c b/arguments.c index 4fc83a0a..49bc8b2a 100644 --- a/arguments.c +++ b/arguments.c @@ -39,9 +39,9 @@ struct args_entry { }; struct args { - struct args_tree tree; - int argc; - char **argv; + struct args_tree tree; + u_int count; + struct args_value *values; }; static struct args_entry *args_find(struct args *, u_char); @@ -66,17 +66,37 @@ args_find(struct args *args, u_char flag) return (RB_FIND(args_tree, &args->tree, &entry)); } +/* Copy value. */ +static void +args_copy_value(struct args_value *to, struct args_value *from) +{ + to->type = from->type; + switch (from->type) { + case ARGS_NONE: + break; + case ARGS_COMMANDS: + to->cmdlist = from->cmdlist; + to->cmdlist->references++; + break; + case ARGS_STRING: + to->string = xstrdup(from->string); + break; + } +} + /* Get value as string. */ -static char * +static const char * args_value_as_string(struct args_value *value) { switch (value->type) { case ARGS_NONE: - return (xstrdup("")); + return (""); case ARGS_COMMANDS: - return (cmd_list_print(value->cmdlist, 0)); + if (value->cached == NULL) + value->cached = cmd_list_print(value->cmdlist, 0); + return (value->cached); case ARGS_STRING: - return (xstrdup(value->string)); + return (value->string); } } @@ -98,10 +118,9 @@ args_parse(const struct args_parse *parse, struct args_value *values, { struct args *args; u_int i; - struct args_value *value; + struct args_value *value, *new; u_char flag, argument; - const char *found, *string; - char *s; + const char *found, *string, *s; if (count == 0) return (args_create()); @@ -109,11 +128,6 @@ args_parse(const struct args_parse *parse, struct args_value *values, args = args_create(); for (i = 1; i < count; /* nothing */) { value = &values[i]; - - s = args_value_as_string(value); - log_debug("%s: %u = %s", __func__, i, s); - free(s); - if (value->type != ARGS_STRING) break; @@ -143,18 +157,20 @@ args_parse(const struct args_parse *parse, struct args_value *values, args_set(args, flag, NULL); continue; } - if (*string != '\0') - s = xstrdup(string); - else { + new = xcalloc(1, sizeof *value); + if (*string != '\0') { + new->type = ARGS_STRING; + new->string = xstrdup(string); + } else { if (i == count) { args_free(args); return (NULL); } - s = args_value_as_string(&values[i++]); + args_copy_value(new, &values[i++]); } + s = args_value_as_string(new); log_debug("%s: add -%c = %s", __func__, flag, s); - args_set(args, flag, s); - free(s); + args_set(args, flag, new); break; } } @@ -165,13 +181,15 @@ args_parse(const struct args_parse *parse, struct args_value *values, s = args_value_as_string(value); log_debug("%s: %u = %s", __func__, i, s); - cmd_append_argv(&args->argc, &args->argv, s); - free(s); + + args->values = xrecallocarray(args->values, + args->count, args->count + 1, sizeof *args->values); + args_copy_value(&args->values[args->count++], value); } } - if ((parse->lower != -1 && args->argc < parse->lower) || - (parse->upper != -1 && args->argc > parse->upper)) { + if ((parse->lower != -1 && args->count < (u_int)parse->lower) || + (parse->upper != -1 && args->count > (u_int)parse->upper)) { args_free(args); return (NULL); } @@ -192,6 +210,7 @@ args_free_value(struct args_value *value) cmd_list_free(value->cmdlist); break; } + free(value->cached); } /* Free an arguments set. */ @@ -202,14 +221,17 @@ args_free(struct args *args) struct args_entry *entry1; struct args_value *value; struct args_value *value1; + u_int i; - cmd_free_argv(args->argc, args->argv); + for (i = 0; i < args->count; i++) + args_free_value(&args->values[i]); + free(args->values); RB_FOREACH_SAFE(entry, args_tree, &args->tree, entry1) { RB_REMOVE(args_tree, &args->tree, entry); TAILQ_FOREACH_SAFE(value, &entry->values, entry, value1) { TAILQ_REMOVE(&entry->values, value, entry); - free(value->string); + args_free_value(value); free(value); } free(entry); @@ -222,8 +244,16 @@ args_free(struct args *args) void args_vector(struct args *args, int *argc, char ***argv) { - *argc = args->argc; - *argv = cmd_copy_argv(args->argc, args->argv); + struct args_value *value; + u_int i; + + *argc = 0; + *argv = NULL; + + for (i = 0; i < args->count; i++) { + value = &args->values[i]; + cmd_append_argv(argc, argv, args_value_as_string(value)); + } } /* Add to string. */ @@ -245,18 +275,28 @@ args_print_add(char **buf, size_t *len, const char *fmt, ...) free(s); } -/* Add argument to string. */ +/* Add value to string. */ static void -args_print_add_argument(char **buf, size_t *len, const char *argument) +args_print_add_value(char **buf, size_t *len, struct args_value *value) { - char *escaped; + char *expanded = NULL; if (**buf != '\0') args_print_add(buf, len, " "); - escaped = args_escape(argument); - args_print_add(buf, len, "%s", escaped); - free(escaped); + switch (value->type) { + case ARGS_NONE: + break; + case ARGS_COMMANDS: + expanded = cmd_list_print(value->cmdlist, 0); + args_print_add(buf, len, "{ %s }", expanded); + break; + case ARGS_STRING: + expanded = args_escape(value->string); + args_print_add(buf, len, "%s", expanded); + break; + } + free(expanded); } /* Print a set of arguments. */ @@ -265,8 +305,7 @@ args_print(struct args *args) { size_t len; char *buf; - int i; - u_int j; + u_int i, j; struct args_entry *entry; struct args_value *value; @@ -291,13 +330,13 @@ args_print(struct args *args) args_print_add(&buf, &len, " -%c", entry->flag); else args_print_add(&buf, &len, "-%c", entry->flag); - args_print_add_argument(&buf, &len, value->string); + args_print_add_value(&buf, &len, value); } } /* And finally the argument vector. */ - for (i = 0; i < args->argc; i++) - args_print_add_argument(&buf, &len, args->argv[i]); + for (i = 0; i < args->count; i++) + args_print_add_value(&buf, &len, &args->values[i]); return (buf); } @@ -363,10 +402,9 @@ args_has(struct args *args, u_char flag) /* Set argument value in the arguments tree. */ void -args_set(struct args *args, u_char flag, const char *s) +args_set(struct args *args, u_char flag, struct args_value *value) { struct args_entry *entry; - struct args_value *value; entry = args_find(args, flag); if (entry == NULL) { @@ -377,12 +415,8 @@ args_set(struct args *args, u_char flag, const char *s) RB_INSERT(args_tree, &args->tree, entry); } else entry->count++; - - if (s != NULL) { - value = xcalloc(1, sizeof *value); - value->string = xstrdup(s); + if (value != NULL && value->type != ARGS_NONE) TAILQ_INSERT_TAIL(&entry->values, value, entry); - } } /* Get argument value. Will be NULL if it isn't present. */ @@ -422,16 +456,25 @@ args_next(struct args_entry **entry) u_int args_count(struct args *args) { - return (args->argc); + return (args->count); +} + +/* Get argument value. */ +struct args_value * +args_value(struct args *args, u_int idx) +{ + if (idx >= args->count) + return (NULL); + return (&args->values[idx]); } /* Return argument as string. */ const char * args_string(struct args *args, u_int idx) { - if (idx >= (u_int)args->argc) + if (idx >= args->count) return (NULL); - return (args->argv[idx]); + return (args_value_as_string(&args->values[idx])); } /* Get first value in argument. */