neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/32561 )
Change subject: vty: show bug in implicit go_parent_node ...................................................................... vty: show bug in implicit go_parent_node Add test to show a problem in VTY node exiting. Back in 2017 when I introduced VTY config file scopes by indenting [1], I actually mistook the vty->priv for the vty->index that we use everywhere to link to the state for our VTY nodes. The intention was that each VTY node child level has its own object linked to it by the vty->index pointer. When the config file leaves a scope, the vty->index should reflect the parent object. Instead I implemented that for the vty->priv pointer only, but we don't use that. Why did this bug not show? A problem happens only if: - a node that uses vty->index is nested inside a node that also uses vty->index. - config sets parent node attributes after a child node. - there is no legacy vty_go_parent() function that sets the correct index via a switch(). [1] "VTY: implicit node exit by de-indenting, not parent lookup" 4a31ffa2f0097d96201f80305a0495c57552f0ad I24cbb3f6de111f2d31110c3c484c066f1153aac9 Change-Id: I2472daed7436a1947655b06d34eb217e595bc7f3 --- M tests/vty/vty_transcript_test.c M tests/vty/vty_transcript_test.vty 2 files changed, 138 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/61/32561/1 diff --git a/tests/vty/vty_transcript_test.c b/tests/vty/vty_transcript_test.c index 1754b67..ccef927 100644 --- a/tests/vty/vty_transcript_test.c +++ b/tests/vty/vty_transcript_test.c @@ -211,6 +211,9 @@ enum { ATTR_TEST_NODE = _LAST_OSMOVTY_NODE + 1, + NEST_A_NODE, + NEST_B_NODE, + NEST_C_NODE, }; static struct cmd_node attr_test_node = { @@ -315,6 +318,62 @@ return CMD_SUCCESS; } +static struct cmd_node nest_a_node = { + NEST_A_NODE, + "%s(config-a)# ", + 1 +}; + +static struct cmd_node nest_b_node = { + NEST_B_NODE, + "%s(config-b)# ", + 1 +}; + +static struct cmd_node nest_c_node = { + NEST_C_NODE, + "%s(config-c)# ", + 1 +}; + +DEFUN(cfg_nest_a, cfg_nest_a_cmd, + "nest NAME", + "Enter nest level a\n" + "Set a name to mark the node's state\n") +{ + vty->index = talloc_asprintf(root_ctx, argv[0]); + vty->node = NEST_A_NODE; + return CMD_SUCCESS; +} + +DEFUN(cfg_nest_b, cfg_nest_b_cmd, + "nest NAME", + "Enter nest level b\n" + "Set a name to mark the node's state\n") +{ + vty->index = talloc_asprintf(root_ctx, argv[0]); + vty->node = NEST_B_NODE; + return CMD_SUCCESS; +} + +DEFUN(cfg_nest_c, cfg_nest_c_cmd, + "nest NAME", + "Enter nest level c\n" + "Set a name to mark the node's state\n") +{ + vty->index = talloc_asprintf(root_ctx, argv[0]); + vty->node = NEST_C_NODE; + return CMD_SUCCESS; +} + +DEFUN(cfg_nest_state, cfg_nest_state_cmd, + "state", + "Show this node's mark\n") +{ + vty_out(vty, "%s%s", (const char *)vty->index, VTY_NEWLINE); + return CMD_SUCCESS; +} + static void init_vty_cmds(void) { install_element_ve(&single0_cmd); @@ -336,6 +395,17 @@ install_element(ATTR_TEST_NODE, &cfg_app_attr_unbelievable_magnificent_cmd); install_element(ATTR_TEST_NODE, &cfg_app_attr_unbelievable_wonderful_cmd); install_element(ATTR_TEST_NODE, &cfg_attr_hidden_app_attr_unbelievable_cmd); + + install_element(CONFIG_NODE, &cfg_nest_a_cmd); + install_node(&nest_a_node, NULL); + install_element(NEST_A_NODE, &cfg_nest_b_cmd); + install_node(&nest_b_node, NULL); + install_element(NEST_B_NODE, &cfg_nest_c_cmd); + install_node(&nest_c_node, NULL); + + install_element(NEST_A_NODE, &cfg_nest_state_cmd); + install_element(NEST_B_NODE, &cfg_nest_state_cmd); + install_element(NEST_C_NODE, &cfg_nest_state_cmd); } int main(int argc, char **argv) diff --git a/tests/vty/vty_transcript_test.vty b/tests/vty/vty_transcript_test.vty index 7b8241e..79c9f4c 100644 --- a/tests/vty/vty_transcript_test.vty +++ b/tests/vty/vty_transcript_test.vty @@ -177,3 +177,37 @@ [expert-mode] But can be seen in the expert mode vty_transcript_test(config-attr-test)# app-hidden-unbelievable? app-hidden-unbelievable Hidden, but still unbelievable help message + +vty_transcript_test(config-attr-test)# exit + +vty_transcript_test(config)# nest A +vty_transcript_test(config-a)# state +A +vty_transcript_test(config-a)# nest B +vty_transcript_test(config-b)# state +B +vty_transcript_test(config-b)# nest C +vty_transcript_test(config-c)# state +C +vty_transcript_test(config-c)# exit +vty_transcript_test(config-b)# state +C +vty_transcript_test(config-b)# ### ^ EXPECTED ERROR: this should say B +vty_transcript_test(config-b)# exit +vty_transcript_test(config-a)# state +C +vty_transcript_test(config-a)# ### ^ EXPECTED ERROR: this should say A +vty_transcript_test(config-a)# nest B2 +vty_transcript_test(config-b)# state +B2 +vty_transcript_test(config-b)# nest C2 +vty_transcript_test(config-c)# state +C2 +vty_transcript_test(config-c)# exit +vty_transcript_test(config-b)# state +C2 +vty_transcript_test(config-b)# ### ^ EXPECTED ERROR: this should say B2 +vty_transcript_test(config-b)# exit +vty_transcript_test(config-a)# state +C2 +vty_transcript_test(config-a)# ### ^ EXPECTED ERROR: this should say A -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32561 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I2472daed7436a1947655b06d34eb217e595bc7f3 Gerrit-Change-Number: 32561 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofm...@sysmocom.de> Gerrit-MessageType: newchange