The function is_config() returns 0 for CONFIG_NODE. Since that node is a config node, the function is renamed to resolve this. --- src/vty/command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/vty/command.c b/src/vty/command.c index faa7c51..7f502db 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -147,7 +147,7 @@ static int cmp_desc(const void *p, const void *q) return strcmp(a->cmd, b->cmd); }
-static int is_config(struct vty *vty) +static int is_config_child(struct vty *vty) { if (vty->node <= CONFIG_NODE) return 0; @@ -2050,7 +2050,7 @@ cmd_execute_command(vector vline, struct vty *vty, struct cmd_element **cmd,
/* Go to parent for config nodes to attempt to find the right command */ while (ret != CMD_SUCCESS && ret != CMD_WARNING - && is_config(vty)) { + && is_config_child(vty)) { vty_go_parent(vty); ret = cmd_execute_command_real(vline, vty, cmd); tried = 1; @@ -2198,7 +2198,7 @@ int config_from_file(struct vty *vty, FILE * fp) /* Try again with setting node to CONFIG_NODE */ while (ret != CMD_SUCCESS && ret != CMD_WARNING && ret != CMD_ERR_NOTHING_TODO - && vty->node != CONFIG_NODE && is_config(vty)) { + && is_config_child(vty)) { vty_go_parent(vty); ret = cmd_execute_command_strict(vline, vty, NULL); }
Put all 'exit' logic into vty_go_parent() and replace the implementations of 'exit' and 'end' by generic ones that use vty_go_parent(). --- src/vty/command.c | 57 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/vty/command.c b/src/vty/command.c index 7f502db..ae21864 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -1889,10 +1889,22 @@ enum node_type vty_go_parent(struct vty *vty) { assert(vty->node > CONFIG_NODE);
- if (host.app_info->go_parent_cb) - host.app_info->go_parent_cb(vty); - else - vty->node = CONFIG_NODE; + switch (vty->node) { + case CONFIG_NODE: + break; + + case CFG_LOG_NODE: + case VTY_NODE: + vty->node = CONFIG_NODE; + break; + + default: + if (host.app_info->go_parent_cb) + host.app_info->go_parent_cb(vty); + else + vty->node = CONFIG_NODE; + break; + }
return vty->node; } @@ -2266,13 +2278,9 @@ gDEFUN(config_exit, vty->node = ENABLE_NODE; vty_config_unlock(vty); break; - case VTY_NODE: - vty->node = CONFIG_NODE; - break; - case CFG_LOG_NODE: - vty->node = CONFIG_NODE; - break; default: + if (vty->node > CONFIG_NODE) + vty_go_parent (vty); break; } return CMD_SUCCESS; @@ -2282,19 +2290,24 @@ gDEFUN(config_exit, gDEFUN(config_end, config_end_cmd, "end", "End current mode and change to enable mode.") { - switch (vty->node) { - case VIEW_NODE: - case ENABLE_NODE: - /* Nothing to do. */ - break; - case CFG_LOG_NODE: - case CONFIG_NODE: - case VTY_NODE: + enum node_type last_node = CONFIG_NODE; + + if (vty->node > ENABLE_NODE) { + /* Repeatedly call go_parent until a top node is reached. */ + while (vty->node > CONFIG_NODE) { + if (vty->node == last_node) { + /* Ensure termination, this shouldn't happen. */ + break; + } + last_node = vty->node; + vty_go_parent(vty); + } + vty_config_unlock(vty); - vty->node = ENABLE_NODE; - break; - default: - break; + if (vty->node > ENABLE_NODE) + vty->node = ENABLE_NODE; + vty->index = NULL; + vty->index_sub = NULL; } return CMD_SUCCESS; }
On Fri, Sep 06, 2013 at 04:51:59PM +0200, Jacob Erlbeck wrote:
- enum node_type last_node = CONFIG_NODE;
- if (vty->node > ENABLE_NODE) {
I moved the enum in here but that doesn't really change anything. Maybe we should have a <= ENABLE_NODE and exit early? But that is not important.
This adds the vty_install_default() function that is basically the install_default() function plus the registration of the commands 'exit' and 'end'. The latter is only provided in subnodes of ENABLED_NODE and CONFIG_NONE.
The VTY test program is extended to check these commands.
Ticket: OW#952 --- include/osmocom/vty/command.h | 5 ++ src/vty/command.c | 18 ++++-- src/vty/logging_vty.c | 3 +- src/vty/vty.c | 2 +- tests/vty/vty_test.c | 124 +++++++++++++++++++++++++++++++++++++++++ tests/vty/vty_test.ok | 43 ++++++++++++++ 6 files changed, 188 insertions(+), 7 deletions(-)
diff --git a/include/osmocom/vty/command.h b/include/osmocom/vty/command.h index 8fbb482..b3b3029 100644 --- a/include/osmocom/vty/command.h +++ b/include/osmocom/vty/command.h @@ -340,6 +340,11 @@ void install_element(enum node_type, struct cmd_element *); void install_element_ve(struct cmd_element *cmd); void sort_node(void);
+/* This is similar to install_default() but it also creates + * 'exit' and 'end' commands. + */ +void vty_install_default(enum node_type); + /* Concatenates argv[shift] through argv[argc-1] into a single NUL-terminated string with a space between each element (allocated using XMALLOC(MTYPE_TMP)). Returns NULL if shift >= argc. */ diff --git a/src/vty/command.c b/src/vty/command.c index ae21864..658f508 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -3302,6 +3302,18 @@ void install_default(enum node_type node) install_element(node, &show_running_config_cmd); }
+void vty_install_default(enum node_type node) +{ + install_default(node); + + install_element(node, &config_exit_cmd); + + if (node >= CONFIG_NODE) { + /* It's not a top node. */ + install_element(node, &config_end_cmd); + } +} + /** * \brief Write the current running config to a given file * \param[in] vty the vty of the code @@ -3380,8 +3392,7 @@ void cmd_init(int terminal) }
if (terminal) { - install_element(ENABLE_NODE, &config_exit_cmd); - install_default(ENABLE_NODE); + vty_install_default(ENABLE_NODE); install_element(ENABLE_NODE, &config_disable_cmd); install_element(ENABLE_NODE, &config_terminal_cmd); install_element (ENABLE_NODE, ©_runningconfig_startupconfig_cmd); @@ -3395,8 +3406,7 @@ void cmd_init(int terminal) install_element(ENABLE_NODE, &config_terminal_no_length_cmd); install_element(ENABLE_NODE, &echo_cmd);
- install_default(CONFIG_NODE); - install_element(CONFIG_NODE, &config_exit_cmd); + vty_install_default(CONFIG_NODE); }
install_element(CONFIG_NODE, &hostname_cmd); diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c index e17c7a8..64e49d7 100644 --- a/src/vty/logging_vty.c +++ b/src/vty/logging_vty.c @@ -677,8 +677,7 @@ void logging_vty_add_cmds(const struct log_info *cat) install_element_ve(&show_alarms_cmd);
install_node(&cfg_log_node, config_write_log); - install_default(CFG_LOG_NODE); - install_element(CFG_LOG_NODE, &config_end_cmd); + vty_install_default(CFG_LOG_NODE); install_element(CFG_LOG_NODE, &logging_fltr_all_cmd); install_element(CFG_LOG_NODE, &logging_use_clr_cmd); install_element(CFG_LOG_NODE, &logging_prnt_timestamp_cmd); diff --git a/src/vty/vty.c b/src/vty/vty.c index 696766a..8bfc35c 100644 --- a/src/vty/vty.c +++ b/src/vty/vty.c @@ -1753,7 +1753,7 @@ void vty_init(struct vty_app_info *app_info) install_element(ENABLE_NODE, &terminal_monitor_cmd); install_element(ENABLE_NODE, &terminal_no_monitor_cmd);
- install_default(VTY_NODE); + vty_install_default(VTY_NODE); install_element(VTY_NODE, &vty_login_cmd); install_element(VTY_NODE, &no_vty_login_cmd); } diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c index 2a9be84..e54a205 100644 --- a/tests/vty/vty_test.c +++ b/tests/vty/vty_test.c @@ -20,10 +20,19 @@ #include <stdio.h> #include <string.h>
+#include <sys/types.h> +#include <sys/socket.h> +#include <sys/un.h> + #include <osmocom/core/talloc.h> #include <osmocom/core/logging.h> #include <osmocom/core/utils.h> #include <osmocom/vty/misc.h> +#include <osmocom/vty/vty.h> +#include <osmocom/vty/command.h> +#include <osmocom/vty/buffer.h> + +static enum event last_vty_connection_event = -1;
static void test_cmd_string_from_valstr(void) { @@ -43,9 +52,124 @@ static void test_cmd_string_from_valstr(void) talloc_free (cmd); }
+static int do_vty_command(struct vty *vty, const char *cmd) +{ + vector vline; + int ret; + + printf("Going to execute '%s'\n", cmd); + vline = cmd_make_strvec(cmd); + ret = cmd_execute_command(vline, vty, NULL, 0); + cmd_free_strvec(vline); + printf("Returned: %d, Current node: %d '%s'\n", ret, vty->node, cmd_prompt(vty->node)); + return ret; +} + +/* Override the implementation from telnet_interface.c */ +void vty_event(enum event event, int sock, struct vty *vty) +{ + last_vty_connection_event = event; + + fprintf(stderr, "Got VTY event: %d\n", event); +} + +static void test_node_tree_structure(void) +{ + struct vty_app_info vty_info = { + .name = "VtyTest", + .version = 0, + .go_parent_cb = NULL, + .is_config_node = NULL, + }; + + const struct log_info_cat default_categories[] = {}; + + const struct log_info log_info = { + .cat = default_categories, + .num_cat = ARRAY_SIZE(default_categories), + }; + + struct vty *vty; + vector vline; + int sock[2]; + + printf("Going to test VTY node tree structure\n"); + + /* Fake logging. */ + osmo_init_logging(&log_info); + + vty_init(&vty_info); + + logging_vty_add_cmds(&log_info); + + /* Fake connection. */ + socketpair(AF_UNIX, SOCK_STREAM, 0, sock); + + vty = vty_create(sock[0], NULL); + + OSMO_ASSERT(vty != NULL); + + OSMO_ASSERT(do_vty_command(vty, "enable") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + OSMO_ASSERT(do_vty_command(vty, "configure terminal") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "exit") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + OSMO_ASSERT(do_vty_command(vty, "configure terminal") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "end") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + OSMO_ASSERT(do_vty_command(vty, "configure terminal") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "log stderr") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CFG_LOG_NODE); + OSMO_ASSERT(do_vty_command(vty, "exit") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "log stderr") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CFG_LOG_NODE); + OSMO_ASSERT(do_vty_command(vty, "end") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + OSMO_ASSERT(do_vty_command(vty, "configure terminal") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "line vty") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == VTY_NODE); + OSMO_ASSERT(do_vty_command(vty, "exit") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "line vty") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == VTY_NODE); + OSMO_ASSERT(do_vty_command(vty, "end") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + + /* Check searching the parents nodes for matching commands. */ + OSMO_ASSERT(do_vty_command(vty, "configure terminal") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CONFIG_NODE); + OSMO_ASSERT(do_vty_command(vty, "log stderr") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CFG_LOG_NODE); + OSMO_ASSERT(do_vty_command(vty, "line vty") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == VTY_NODE); + OSMO_ASSERT(do_vty_command(vty, "log stderr") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == CFG_LOG_NODE); + OSMO_ASSERT(do_vty_command(vty, "end") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + + /* Check for final 'exit' (connection close). */ + OSMO_ASSERT(do_vty_command(vty, "exit") == CMD_SUCCESS); + OSMO_ASSERT(vty->node == ENABLE_NODE); + OSMO_ASSERT(vty->status == VTY_CLOSE); + + vty_close(vty); + OSMO_ASSERT(last_vty_connection_event == VTY_CLOSED); +} + int main(int argc, char **argv) { test_cmd_string_from_valstr(); + test_node_tree_structure(); printf("All tests passed\n");
return 0; diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok index baec249..0ea2dab 100644 --- a/tests/vty/vty_test.ok +++ b/tests/vty/vty_test.ok @@ -1,3 +1,46 @@ Going to test vty_cmd_string_from_valstr() Tested with %s-strings, resulting cmd = '[prefix%s%s%s%s%s][foo%s%s%s%s%s][sep%s%s%s%s%s][bar%s%s%s%s%s][end%s%s%s%s%s]' +Going to test VTY node tree structure +Going to execute 'enable' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'configure terminal' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'exit' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'configure terminal' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'end' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'configure terminal' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'log stderr' +Returned: 0, Current node: 7 '%s(config-log)# ' +Going to execute 'exit' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'log stderr' +Returned: 0, Current node: 7 '%s(config-log)# ' +Going to execute 'end' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'configure terminal' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'line vty' +Returned: 0, Current node: 8 '%s(config-line)# ' +Going to execute 'exit' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'line vty' +Returned: 0, Current node: 8 '%s(config-line)# ' +Going to execute 'end' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'configure terminal' +Returned: 0, Current node: 4 '%s(config)# ' +Going to execute 'log stderr' +Returned: 0, Current node: 7 '%s(config-log)# ' +Going to execute 'line vty' +Returned: 0, Current node: 8 '%s(config-line)# ' +Going to execute 'log stderr' +Returned: 0, Current node: 7 '%s(config-log)# ' +Going to execute 'end' +Returned: 0, Current node: 3 '%s# ' +Going to execute 'exit' +Returned: 0, Current node: 3 '%s# ' All tests passed
On Fri, Sep 06, 2013 at 04:51:58PM +0200, Jacob Erlbeck wrote:
Very nice patch set!
thanks a lot. I am applying and pushing it now.
holger