[PATCH] libosmocore[master]: VTY: implicit node exit by de-indenting, not parent lookup
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/3880 to look at the new patch set (#4). VTY: implicit node exit by de-indenting, not parent lookup Note: This will break users' config files if they do not use consistent indenting. (see below for a definition of "consistent".) When reading VTY commands from a file, use indenting as means to implicitly exit child nodes. Do not look for commands in the parent node implicitly. The VTY so far implies 'exit' commands if a VTY line cannot be parsed on the current node, but succeeds on the parent node. That is the mechanism by which our VTY config files do not need 'exit' at the end of each child node. We've hit problems with this in the following scenarios, which will show improved user experience after this patch: *) When both a parent and its child node have commands with identical names: cs7 instace 0 point-code 1.2.3 sccp-address osmo-msc point-code 0.0.1 If I put the parent's command below the child, it is still interpreted in the context of the child node: cs7 instace 0 sccp-address osmo-msc point-code 0.0.1 point-code 1.2.3 Though the indenting lets me assume I am setting the cs7 instance's global PC to 1.2.3, I'm actually overwriting osmo-msc's PC with 1.2.3 and discarding the 0.0.1. *) When a software change moves a VTY command from a child to a parent. Say 'timezone' moved from 'bts' to 'network' level: network timezone 1 2 Say a user still has an old config file with 'timezone' on the child level: network bts 0 timezone 1 2 trx 0 The user would expect an error message that 'timezone' is invalid on the 'bts' level. Instead, the VTY finds the parent node's 'timezone', steps out of 'bts' to the 'network' level, and instead says that the 'trx' command does not exist. Format: Consistent means that two adjacent indenting lines have the exact same indenting characters for the common length: Weird mix if you ask me, but correct and consistent: ROOT PARENT CHILD GRANDCHILD GRANDCHILD2 SIBLING Inconsistent: ROOT PARENT CHILD GRANDCHILD GRANDCHILD2 SIBLING Also, when going back to a parent level, the exact same indenting must be used as before in that node: Incorrect: ROOT PARENT CHILD SIBLING As not really intended side effect, it is also permitted to indent the entire file starting from the root level. We could guard against it but there's no harm: Correct and consistent: ROOT PARENT CHILD SIBLING Implementation: Track parent nodes state: whenever a command enters a child node, push a parent node onto an llist to remember the exact indentation characters used for that level. As soon as the first line on a child node is parsed, remember this new indentation (which must have a longer strlen() than its parent level) to apply to all remaining child siblings and grandchildren. If the amount of spaces that indent a following VTY command are less than this expected indentation, call vty_go_parent() until it matches up. At any level, if the common length of indentation characters mismatch, abort parsing in error. Transitions to child node are spread across VTY implementations and are hard to change. But transitions to the parent node are all handled by vty_go_parent(). By popping a parent from the list of parents in vty_go_parent(), we can also detect that a command has changed the node without changing the parent, hence it must have stepped into a child node, and we can push a parent frame. The behavior on the interactive telnet VTY remains unchanged. Change-Id: I24cbb3f6de111f2d31110c3c484c066f1153aac9 --- M include/osmocom/vty/command.h M include/osmocom/vty/vty.h M src/vty/command.c M src/vty/vty.c M tests/Makefile.am M tests/testsuite.at A tests/vty/fail_not_de-indented.cfg A tests/vty/fail_tabs_and_spaces.cfg A tests/vty/fail_too_much_indent.cfg A tests/vty/ok.cfg A tests/vty/ok_ignore_blank.cfg A tests/vty/ok_ignore_comment.cfg A tests/vty/ok_indented_root.cfg A tests/vty/ok_more_spaces.cfg A tests/vty/ok_tabs.cfg A tests/vty/ok_tabs_and_spaces.cfg M tests/vty/vty_test.c M tests/vty/vty_test.ok 18 files changed, 321 insertions(+), 26 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/3880/4 diff --git a/include/osmocom/vty/command.h b/include/osmocom/vty/command.h index 0fa5175..cb2edaa 100644 --- a/include/osmocom/vty/command.h +++ b/include/osmocom/vty/command.h @@ -161,6 +161,7 @@ #define CMD_COMPLETE_MATCH 8 #define CMD_COMPLETE_LIST_MATCH 9 #define CMD_SUCCESS_DAEMON 10 +#define CMD_ERR_INVALID_INDENT 11 /* Argc max counts. */ #define CMD_ARGC_MAX 256 @@ -368,6 +369,7 @@ char *argv_concat(const char **argv, int argc, int shift); vector cmd_make_strvec(const char *); +int cmd_make_strvec2(const char *string, char **indent, vector *strvec_p); void cmd_free_strvec(vector); vector cmd_describe_command(); char
[PATCH] libosmocore[master]: VTY: implicit node exit by de-indenting, not parent lookup
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/3880 to look at the new patch set (#2). VTY: implicit node exit by de-indenting, not parent lookup Note: This will break users' config files if they do not use consistent indenting. (see below for a definition of "consistent".) When reading VTY commands from a file, use indenting as means to implicitly exit child nodes. Do not look for commands in the parent node implicitly. The VTY so far implies 'exit' commands if a VTY line cannot be parsed on the current node, but succeeds on the parent node. That is the mechanism by which our VTY config files do not need 'exit' at the end of each child node. We've hit problems with this in the following scenarios, which will show improved user experience after this patch: *) When both a parent and its child node have commands with identical names: cs7 instace 0 point-code 1.2.3 sccp-address osmo-msc point-code 0.0.1 If I put the parent's command below the child, it is still interpreted in the context of the child node: cs7 instace 0 sccp-address osmo-msc point-code 0.0.1 point-code 1.2.3 Though the indenting lets me assume I am setting the cs7 instance's global PC to 1.2.3, I'm actually overwriting osmo-msc's PC with 1.2.3 and discarding the 0.0.1. *) When a software change moves a VTY command from a child to a parent. Say 'timezone' moved from 'bts' to 'network' level: network timezone 1 2 Say a user still has an old config file with 'timezone' on the child level: network bts 0 timezone 1 2 trx 0 The user would expect an error message that 'timezone' is invalid on the 'bts' level. Instead, the VTY finds the parent node's 'timezone', steps out of 'bts' to the 'network' level, and instead says that the 'trx' command does not exist. Format: Consistent means that two adjacent indenting lines have the exact same indenting characters for the common length: Weird mix if you ask me, but correct and consistent: ROOT PARENT CHILD GRANDCHILD GRANDCHILD2 SIBLING Inconsistent: ROOT PARENT CHILD GRANDCHILD GRANDCHILD2 SIBLING Also, when going back to a parent level, the exact same indenting must be used as before in that node: Incorrect: ROOT PARENT CHILD SIBLING As not really intended side effect, it is also permitted to indent the entire file starting from the root level. We could guard against it but there's no harm: Correct and consistent: ROOT PARENT CHILD SIBLING Implementation: Track parent nodes state: whenever a command enters a child node, push a parent node onto an llist to remember the exact indentation characters used for that level. As soon as the first line on a child node is parsed, remember this new indentation (which must have a longer strlen() than its parent level) to apply to all remaining child siblings and grandchildren. If the amount of spaces that indent a following VTY command are less than this expected indentation, call vty_go_parent() until it matches up. At any level, if the common length of indentation characters mismatch, abort parsing in error. Transitions to child node are spread across VTY implementations and are hard to change. But transitions to the parent node are all handled by vty_go_parent(). By popping a parent from the list of parents in vty_go_parent(), we can also detect that a command has changed the node without changing the parent, hence it must have stepped into a child node, and we can push a parent frame. The behavior on the interactive telnet VTY remains unchanged. Change-Id: I24cbb3f6de111f2d31110c3c484c066f1153aac9 --- M include/osmocom/vty/command.h M include/osmocom/vty/vty.h M src/vty/command.c M src/vty/vty.c M tests/Makefile.am M tests/testsuite.at A tests/vty/fail_not_de-indented.cfg A tests/vty/fail_tabs_and_spaces.cfg A tests/vty/fail_too_much_indent.cfg A tests/vty/ok.cfg A tests/vty/ok_ignore_blank.cfg A tests/vty/ok_ignore_comment.cfg A tests/vty/ok_indented_root.cfg A tests/vty/ok_more_spaces.cfg A tests/vty/ok_tabs.cfg A tests/vty/ok_tabs_and_spaces.cfg M tests/vty/vty_test.c M tests/vty/vty_test.ok 18 files changed, 312 insertions(+), 26 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/80/3880/2 diff --git a/include/osmocom/vty/command.h b/include/osmocom/vty/command.h index 0fa5175..cb2edaa 100644 --- a/include/osmocom/vty/command.h +++ b/include/osmocom/vty/command.h @@ -161,6 +161,7 @@ #define CMD_COMPLETE_MATCH 8 #define CMD_COMPLETE_LIST_MATCH 9 #define CMD_SUCCESS_DAEMON 10 +#define CMD_ERR_INVALID_INDENT 11 /* Argc max counts. */ #define CMD_ARGC_MAX 256 @@ -368,6 +369,7 @@ char *argv_concat(const char **argv, int argc, int shift); vector cmd_make_strvec(const char *); +int cmd_make_strvec2(const char *string, char **indent, vector *strvec_p); void cmd_free_strvec(vector); vector cmd_describe_command(); char
