From 53b5bae41b58616d17980b0620cef0a0c584c18b Mon Sep 17 00:00:00 2001 From: Krzysztof Chruscinski Date: Wed, 8 Apr 2020 15:42:06 +0200 Subject: [PATCH] shell: Refactor command getters Refactor and simplified fetching commands from the tree of commands. Signed-off-by: Krzysztof Chruscinski --- subsys/shell/shell.c | 75 ++++++++++---------------- subsys/shell/shell_cmds.c | 8 ++- subsys/shell/shell_help.c | 46 +++++----------- subsys/shell/shell_utils.c | 99 ++++++++++++++--------------------- subsys/shell/shell_utils.h | 25 ++++----- subsys/shell/shell_wildcard.c | 46 ++++------------ subsys/shell/shell_wildcard.h | 4 +- 7 files changed, 108 insertions(+), 195 deletions(-) diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index 227bb48eee9..88885da761e 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -252,7 +252,7 @@ static bool tab_prepare(const struct shell *shell, /* root command completion */ if ((*argc == 0) || ((space == 0) && (*argc == 1))) { *complete_arg_idx = SHELL_CMD_ROOT_LVL; - *cmd = NULL; + *cmd = shell->ctx->selected_cmd; return true; } @@ -294,39 +294,25 @@ static void find_completion_candidates(const struct shell *shell, { size_t incompl_cmd_len = shell_strlen(incompl_cmd); const struct shell_static_entry *candidate; - struct shell_static_entry dynamic_entry; - bool found = false; + struct shell_static_entry dloc; size_t idx = 0; *longest = 0U; *cnt = 0; - while (true) { - bool is_empty; + while ((candidate = shell_cmd_get(cmd, idx, &dloc)) != NULL) { bool is_candidate; - shell_cmd_get(shell, cmd ? cmd->subcmd : NULL, cmd ? 1 : 0, - idx, &candidate, &dynamic_entry); - - if (!candidate) { - break; - } - - is_empty = is_empty_cmd(candidate); is_candidate = is_completion_candidate(candidate->syntax, incompl_cmd, incompl_cmd_len); - if (!is_empty && is_candidate) { - size_t slen = strlen(candidate->syntax); - - *longest = (slen > *longest) ? slen : *longest; - (*cnt)++; - - if (!found) { + if (!is_empty_cmd(candidate) && is_candidate) { + *longest = Z_MAX(strlen(candidate->syntax), *longest); + if (*cnt == 0) { *first_idx = idx; } - - found = true; + (*cnt)++; } + idx++; } } @@ -343,8 +329,8 @@ static void autocomplete(const struct shell *shell, /* shell->ctx->active_cmd can be safely used outside of command context * to save stack */ - shell_cmd_get(shell, cmd ? cmd->subcmd : NULL, cmd ? 1 : 0, - subcmd_idx, &match, &shell->ctx->active_cmd); + match = shell_cmd_get(cmd, subcmd_idx, &shell->ctx->active_cmd); + __ASSERT_NO_MSG(match != NULL); cmd_len = shell_strlen(match->syntax); /* no exact match found */ @@ -408,8 +394,8 @@ static void tab_options_print(const struct shell *shell, /* shell->ctx->active_cmd can be safely used outside of command * context to save stack */ - shell_cmd_get(shell, cmd ? cmd->subcmd : NULL, cmd ? 1 : 0, - idx, &match, &shell->ctx->active_cmd); + match = shell_cmd_get(cmd, idx, &shell->ctx->active_cmd); + __ASSERT_NO_MSG(match != NULL); idx++; is_empty = is_empty_cmd(match); if (is_empty || (str && match->syntax && @@ -437,8 +423,8 @@ static u16_t common_beginning_find(const struct shell *shell, __ASSERT_NO_MSG(cnt > 1); - shell_cmd_get(shell, cmd ? cmd->subcmd : NULL, cmd ? 1 : 0, - first, &match, &dynamic_entry); + match = shell_cmd_get(cmd, first, &dynamic_entry); + __ASSERT_NO_MSG(match); strncpy(shell->ctx->temp_buff, match->syntax, sizeof(shell->ctx->temp_buff) - 1); @@ -449,9 +435,7 @@ static u16_t common_beginning_find(const struct shell *shell, const struct shell_static_entry *match2; int curr_common; - shell_cmd_get(shell, cmd ? cmd->subcmd : NULL, cmd ? 1 : 0, - idx++, &match2, &dynamic_entry2); - + match2 = shell_cmd_get(cmd, idx++, &dynamic_entry2); if (match2 == NULL) { break; } @@ -535,10 +519,10 @@ static int exec_cmd(const struct shell *shell, size_t argc, char **argv, */ static int execute(const struct shell *shell) { - struct shell_static_entry d_entry; /* Memory for dynamic commands. */ + struct shell_static_entry dloc; /* Memory for dynamic commands. */ char *argv[CONFIG_SHELL_ARGC_MAX + 1]; /* +1 reserved for NULL */ - const struct shell_static_entry *p_static_entry = NULL; - const struct shell_cmd_entry *p_cmd = NULL; + const struct shell_static_entry *parent = shell->ctx->selected_cmd; + const struct shell_static_entry *entry = NULL; struct shell_static_entry help_entry; size_t cmd_lvl = SHELL_CMD_ROOT_LVL; size_t cmd_with_handler_lvl = 0; @@ -607,7 +591,7 @@ static int execute(const struct shell *shell) if (IS_ENABLED(CONFIG_SHELL_WILDCARD) && (cmd_lvl > 0)) { enum shell_wildcard_status status; - status = shell_wildcard_process(shell, p_cmd, + status = shell_wildcard_process(shell, entry, argv[cmd_lvl]); /* Wildcard character found but there is no matching * command. @@ -626,10 +610,9 @@ static int execute(const struct shell *shell) } } - shell_cmd_get(shell, p_cmd, cmd_lvl, cmd_idx++, &p_static_entry, - &d_entry); + entry = shell_cmd_get(parent, cmd_idx++, &dloc); - if ((cmd_idx == 0) || (p_static_entry == NULL)) { + if ((cmd_idx == 0) || (entry == NULL)) { if (cmd_lvl == 0 && (!shell_in_select_mode(shell) || shell->ctx->selected_cmd->handler == NULL)) { @@ -640,16 +623,16 @@ static int execute(const struct shell *shell) } if (shell_in_select_mode(shell) && shell->ctx->selected_cmd->handler != NULL) { - p_static_entry = shell->ctx->selected_cmd; - shell->ctx->active_cmd = *p_static_entry; + entry = shell->ctx->selected_cmd; + shell->ctx->active_cmd = *entry; cmd_with_handler_lvl = cmd_lvl; } break; } - if (strcmp(argv[cmd_lvl], p_static_entry->syntax) == 0) { + if (strcmp(argv[cmd_lvl], entry->syntax) == 0) { /* checking if command has a handler */ - if (p_static_entry->handler != NULL) { + if (entry->handler != NULL) { if (IS_ENABLED(CONFIG_SHELL_WILDCARD) && (wildcard_found)) { shell_op_cursor_end_move(shell); @@ -668,17 +651,17 @@ static int execute(const struct shell *shell) return -ENOEXEC; } - shell->ctx->active_cmd = *p_static_entry; + shell->ctx->active_cmd = *entry; cmd_with_handler_lvl = cmd_lvl; } /* checking if function has a help handler */ - if (p_static_entry->help != NULL) { - help_entry = *p_static_entry; + if (entry->help != NULL) { + help_entry = *entry; } cmd_lvl++; cmd_idx = 0; - p_cmd = p_static_entry->subcmd; + parent = entry; } } diff --git a/subsys/shell/shell_cmds.c b/subsys/shell/shell_cmds.c index a580a3b7b5b..6dd5e384e05 100644 --- a/subsys/shell/shell_cmds.c +++ b/subsys/shell/shell_cmds.c @@ -380,6 +380,11 @@ static int cmd_resize(const struct shell *shell, size_t argc, char **argv) return 0; } +static bool no_args(const struct shell_static_entry *entry) +{ + return (entry->args.mandatory == 1) && (entry->args.optional == 0); +} + static int cmd_select(const struct shell *shell, size_t argc, char **argv) { const struct shell_static_entry *candidate = NULL; @@ -391,7 +396,8 @@ static int cmd_select(const struct shell *shell, size_t argc, char **argv) candidate = shell_get_last_command(shell, argc, argv, &matching_argc, &entry, true); - if ((candidate != NULL) && (argc == matching_argc)) { + if ((candidate != NULL) && !no_args(candidate) + && (argc == matching_argc)) { shell->ctx->selected_cmd = candidate; return 0; } diff --git a/subsys/shell/shell_help.c b/subsys/shell/shell_help.c index 207869309e9..237bda9242b 100644 --- a/subsys/shell/shell_help.c +++ b/subsys/shell/shell_help.c @@ -148,50 +148,28 @@ static void help_item_print(const struct shell *shell, const char *item_name, void shell_help_subcmd_print(const struct shell *shell) { const struct shell_static_entry *entry = NULL; - struct shell_static_entry static_entry; - u16_t longest_syntax = 0U; - size_t cmd_idx = 0; - - /* Checking if there are any subcommands available. */ - if (!shell->ctx->active_cmd.subcmd) { - return; - } + const struct shell_static_entry *parent = &shell->ctx->active_cmd; + struct shell_static_entry dloc; + u16_t longest = 0U; + size_t idx = 0; /* Searching for the longest subcommand to print. */ - do { - shell_cmd_get(shell, shell->ctx->active_cmd.subcmd, - !SHELL_CMD_ROOT_LVL, - cmd_idx++, &entry, &static_entry); + while ((entry = shell_cmd_get(parent, idx++, &dloc)) != NULL) { + longest = Z_MAX(longest, shell_strlen(entry->syntax)); + }; - if (!entry) { - break; - } - - u16_t len = shell_strlen(entry->syntax); - - longest_syntax = longest_syntax > len ? longest_syntax : len; - } while (cmd_idx != 0); /* too many commands */ - - if (cmd_idx == 1) { + /* No help to print */ + if (longest == 0) { return; } shell_internal_fprintf(shell, SHELL_NORMAL, "Subcommands:\n"); /* Printing subcommands and help string (if exists). */ - cmd_idx = 0; + idx = 0; - while (true) { - shell_cmd_get(shell, shell->ctx->active_cmd.subcmd, - !SHELL_CMD_ROOT_LVL, - cmd_idx++, &entry, &static_entry); - - if (entry == NULL) { - break; - } - - help_item_print(shell, entry->syntax, longest_syntax, - entry->help); + while ((entry = shell_cmd_get(parent, idx++, &dloc)) != NULL) { + help_item_print(shell, entry->syntax, longest, entry->help); } } diff --git a/subsys/shell/shell_utils.c b/subsys/shell/shell_utils.c index b53231d5a9e..3f8a6e34391 100644 --- a/subsys/shell/shell_utils.c +++ b/subsys/shell/shell_utils.c @@ -12,7 +12,7 @@ extern const struct shell_cmd_entry __shell_root_cmds_end[0]; static inline const struct shell_cmd_entry *shell_root_cmd_get(u32_t id) { - return &__shell_root_cmds_start[id]; + return &__shell_root_cmds_start[id]; } /* Calculates relative line number of given position in buffer */ @@ -225,7 +225,7 @@ static inline u32_t shell_root_cmd_count(void) sizeof(struct shell_cmd_entry); } -/* Function returning pointer to root command matching requested syntax. */ +/* Function returning pointer to parent command matching requested syntax. */ const struct shell_static_entry *shell_root_cmd_find(const char *syntax) { const size_t cmd_count = shell_root_cmd_count(); @@ -241,46 +241,33 @@ const struct shell_static_entry *shell_root_cmd_find(const char *syntax) return NULL; } -void shell_cmd_get(const struct shell *shell, - const struct shell_cmd_entry *command, size_t lvl, - size_t idx, const struct shell_static_entry **entry, - struct shell_static_entry *d_entry) +const struct shell_static_entry *shell_cmd_get( + const struct shell_static_entry *parent, + size_t idx, + struct shell_static_entry *dloc) { - __ASSERT_NO_MSG(entry != NULL); - __ASSERT_NO_MSG(d_entry != NULL); + __ASSERT_NO_MSG(dloc != NULL); + const struct shell_static_entry *res = NULL; - *entry = NULL; + if (parent == NULL) { + return (idx < shell_root_cmd_count()) ? + shell_root_cmd_get(idx)->u.entry : NULL; + } - if (lvl == SHELL_CMD_ROOT_LVL) { - if (shell_in_select_mode(shell)) { - const struct shell_static_entry *ptr = - shell->ctx->selected_cmd; - if (ptr->subcmd->u.entry[idx].syntax != NULL) { - *entry = &ptr->subcmd->u.entry[idx]; + if (parent->subcmd) { + if (parent->subcmd->is_dynamic) { + parent->subcmd->u.dynamic_get(idx, dloc); + if (dloc->syntax != NULL) { + res = dloc; + } + } else { + if (parent->subcmd->u.entry[idx].syntax != NULL) { + res = &parent->subcmd->u.entry[idx]; } - } else if (idx < shell_root_cmd_count()) { - const struct shell_cmd_entry *cmd; - - cmd = shell_root_cmd_get(idx); - *entry = cmd->u.entry; - } - return; - } - - if (command == NULL) { - return; - } - - if (command->is_dynamic) { - command->u.dynamic_get(idx, d_entry); - if (d_entry->syntax != NULL) { - *entry = d_entry; - } - } else { - if (command->u.entry[idx].syntax != NULL) { - *entry = &command->u.entry[idx]; } } + + return res; } /* Function returns pointer to a command matching given pattern. @@ -288,28 +275,26 @@ void shell_cmd_get(const struct shell *shell, * @param shell Shell instance. * @param cmd Pointer to commands array that will be searched. * @param lvl Root command indicator. If set to 0 shell will search - * for pattern in root commands. + * for pattern in parent commands. * @param cmd_str Command pattern to be found. - * @param d_entry Shell static command descriptor. + * @param dloc Shell static command descriptor. * * @return Pointer to found command. */ static const struct shell_static_entry *find_cmd( - const struct shell *shell, - const struct shell_cmd_entry *cmd, - size_t lvl, - char *cmd_str, - struct shell_static_entry *d_entry) + const struct shell *shell, + const struct shell_static_entry *parent, + char *cmd_str, + struct shell_static_entry *dloc) { - const struct shell_static_entry *entry = NULL; + const struct shell_static_entry *entry; size_t idx = 0; - do { - shell_cmd_get(shell, cmd, lvl, idx++, &entry, d_entry); - if (entry && (strcmp(cmd_str, entry->syntax) == 0)) { + while ((entry = shell_cmd_get(parent, idx++, dloc)) != NULL) { + if (strcmp(cmd_str, entry->syntax) == 0) { return entry; } - } while (entry); + }; return NULL; } @@ -319,12 +304,11 @@ const struct shell_static_entry *shell_get_last_command( size_t argc, char *argv[], size_t *match_arg, - struct shell_static_entry *d_entry, + struct shell_static_entry *dloc, bool only_static) { const struct shell_static_entry *prev_entry = NULL; - const struct shell_static_entry *entry = NULL; - const struct shell_cmd_entry *cmd = NULL; + const struct shell_static_entry *entry = shell->ctx->selected_cmd; *match_arg = SHELL_CMD_ROOT_LVL; @@ -338,21 +322,16 @@ const struct shell_static_entry *shell_get_last_command( } } - entry = find_cmd(shell, cmd, *match_arg, argv[*match_arg], - d_entry); + prev_entry = entry; + entry = find_cmd(shell, entry, argv[*match_arg], dloc); if (entry) { - cmd = entry->subcmd; - prev_entry = entry; (*match_arg)++; } else { + entry = prev_entry; break; } - if (cmd == NULL) { - return NULL; - } - - if (only_static && cmd->is_dynamic) { + if (only_static && (entry == dloc)) { (*match_arg)--; return NULL; } diff --git a/subsys/shell/shell_utils.h b/subsys/shell/shell_utils.h index 1ff1e6a46d2..71dd66ea6bd 100644 --- a/subsys/shell/shell_utils.h +++ b/subsys/shell/shell_utils.h @@ -43,25 +43,18 @@ char shell_make_argv(size_t *argc, char **argv, char *cmd, uint8_t max_argc); */ void shell_pattern_remove(char *buff, u16_t *buff_len, const char *pattern); -/* @brief Function shall be used to search commands. +/** @brief Get subcommand with given index from the root. * - * It moves the pointer entry to command of static command structure. If the - * command cannot be found, the function will set entry to NULL. + * @param parent Parent entry. Null to get root command from index. + * @param idx Command index. + * @param dloc Location used to write dynamic entry. * - * @param shell Shell instance. - * @param command Pointer to command which will be processed (no matter - * the root command). - * @param lvl Level of the requested command. - * @param idx Index of the requested command. - * @param entry Pointer which points to subcommand[idx] after function - * execution. - * @param st_entry Pointer to the structure where dynamic entry data can - * be stored. + * @return Fetched command or null if command with that index does not exist. */ -void shell_cmd_get(const struct shell *shell, - const struct shell_cmd_entry *command, size_t lvl, - size_t idx, const struct shell_static_entry **entry, - struct shell_static_entry *d_entry); +const struct shell_static_entry *shell_cmd_get( + const struct shell_static_entry *parent, + size_t idx, + struct shell_static_entry *dloc); /* @internal @brief Function returns pointer to a shell's subcommands array diff --git a/subsys/shell/shell_wildcard.c b/subsys/shell/shell_wildcard.c index 59bf52457e4..871f5d6f4d9 100644 --- a/subsys/shell/shell_wildcard.c +++ b/subsys/shell/shell_wildcard.c @@ -10,27 +10,6 @@ #include "shell_utils.h" #include "shell_ops.h" -static void subcmd_get(const struct shell_cmd_entry *cmd, - size_t idx, const struct shell_static_entry **entry, - struct shell_static_entry *d_entry) -{ - __ASSERT_NO_MSG(entry != NULL); - __ASSERT_NO_MSG(d_entry != NULL); - - if (cmd == NULL) { - *entry = NULL; - return; - } - - if (cmd->is_dynamic) { - cmd->u.dynamic_get(idx, d_entry); - *entry = (d_entry->syntax != NULL) ? d_entry : NULL; - } else { - *entry = (cmd->u.entry[idx].syntax != NULL) ? - &cmd->u.entry[idx] : NULL; - } -} - static enum shell_wildcard_status command_add(char *buff, u16_t *buff_len, char const *cmd, char const *pattern) @@ -88,26 +67,21 @@ static enum shell_wildcard_status command_add(char *buff, u16_t *buff_len, * @retval WILDCARD_CMD_NO_MATCH_FOUND No matching command found. */ static enum shell_wildcard_status commands_expand(const struct shell *shell, - const struct shell_cmd_entry *cmd, - const char *pattern) + const struct shell_static_entry *cmd, + const char *pattern) { enum shell_wildcard_status ret_val = SHELL_WILDCARD_CMD_NO_MATCH_FOUND; - struct shell_static_entry const *p_static_entry = NULL; - struct shell_static_entry static_entry; + struct shell_static_entry const *entry = NULL; + struct shell_static_entry dloc; size_t cmd_idx = 0; size_t cnt = 0; - do { - subcmd_get(cmd, cmd_idx++, &p_static_entry, &static_entry); + while ((entry = shell_cmd_get(cmd, cmd_idx++, &dloc)) != NULL) { - if (!p_static_entry) { - break; - } - - if (fnmatch(pattern, p_static_entry->syntax, 0) == 0) { + if (fnmatch(pattern, entry->syntax, 0) == 0) { ret_val = command_add(shell->ctx->temp_buff, &shell->ctx->cmd_tmp_buff_len, - p_static_entry->syntax, pattern); + entry->syntax, pattern); if (ret_val == SHELL_WILDCARD_CMD_MISSING_SPACE) { shell_internal_fprintf(shell, SHELL_WARNING, @@ -121,7 +95,7 @@ static enum shell_wildcard_status commands_expand(const struct shell *shell, } cnt++; } - } while (cmd_idx); + }; if (cnt > 0) { shell_pattern_remove(shell->ctx->temp_buff, @@ -185,8 +159,8 @@ void shell_wildcard_prepare(const struct shell *shell) enum shell_wildcard_status shell_wildcard_process(const struct shell *shell, - const struct shell_cmd_entry *cmd, - const char *pattern) + const struct shell_static_entry *cmd, + const char *pattern) { enum shell_wildcard_status ret_val = SHELL_WILDCARD_NOT_FOUND; diff --git a/subsys/shell/shell_wildcard.h b/subsys/shell/shell_wildcard.h index 47d776c3e71..9e7a7faa5d8 100644 --- a/subsys/shell/shell_wildcard.h +++ b/subsys/shell/shell_wildcard.h @@ -45,8 +45,8 @@ bool shell_wildcard_character_exist(const char *str); * @param[in] pattern Pointer to wildcard pattern. */ enum shell_wildcard_status shell_wildcard_process(const struct shell *shell, - const struct shell_cmd_entry *cmd, - const char *pattern); + const struct shell_static_entry *cmd, + const char *pattern); /* Function finalizing wildcard expansion procedure. *