In this case the last_node variable may hold values that are not in enum node_type, so int is used instead. --- src/vty/command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vty/command.c b/src/vty/command.c index df2ffea..7f76ced 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -2291,7 +2291,7 @@ gDEFUN(config_exit, config_end_cmd, "end", "End current mode and change to enable mode.") { if (vty->node > ENABLE_NODE) { - enum node_type last_node = CONFIG_NODE; + int last_node = CONFIG_NODE;
/* Repeatedly call go_parent until a top node is reached. */ while (vty->node > CONFIG_NODE) {
This patch removes an assertion of node > CONFIG_NODE and changes the function to handle all nodes properly. For the sake of completeness, the generic exit command implementation is extended to work properly with all nodes, too. --- src/vty/command.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/vty/command.c b/src/vty/command.c index 7f76ced..38a32cf 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -29,7 +29,6 @@ Boston, MA 02111-1307, USA. */ #include <errno.h> #define _XOPEN_SOURCE #include <unistd.h> -#include <assert.h> #include <ctype.h> #include <time.h> #include <sys/time.h> @@ -1884,21 +1883,31 @@ char **cmd_complete_command(vector vline, struct vty *vty, int *status) }
/* return parent node */ -/* MUST eventually converge on CONFIG_NODE */ +/* + * MUST eventually converge either on CONFIG_NODE for every config node or + * on CONFIG_ENABLE for every other user defined node. + */ enum node_type vty_go_parent(struct vty *vty) { - assert(vty->node > CONFIG_NODE); - switch (vty->node) { + case AUTH_NODE: + case VIEW_NODE: + case ENABLE_NODE: case CONFIG_NODE: break;
+ case AUTH_ENABLE_NODE: + vty->node = VIEW_NODE; + break; + case CFG_LOG_NODE: case VTY_NODE: vty->node = CONFIG_NODE; break;
default: + if (vty->node > CONFIG_NODE); + if (host.app_info->go_parent_cb) host.app_info->go_parent_cb(vty); else @@ -2267,6 +2276,7 @@ gDEFUN(config_exit, config_exit_cmd, "exit", "Exit current mode and down to previous mode\n") { switch (vty->node) { + case AUTH_NODE: case VIEW_NODE: case ENABLE_NODE: if (0) //vty_shell (vty))
On Tue, Sep 10, 2013 at 09:07:32AM +0200, Jacob Erlbeck wrote:
/* return parent node */ -/* MUST eventually converge on CONFIG_NODE */ +/*
- MUST eventually converge either on CONFIG_NODE for every config node or
- on CONFIG_ENABLE for every other user defined node.
- */
The comment is wrong. There is no CONFIG_ENABLE node but the comment sounds dangerous too. The ENABLE_NODE might be password protected and I would like to avoid a situation where we come from a 'child' of the VIEW_NODE and end in 'ENABLE_NODE'.
On 09/10/2013 11:06 AM, Holger Hans Peter Freyther wrote:
On Tue, Sep 10, 2013 at 09:07:32AM +0200, Jacob Erlbeck wrote:
/* return parent node */ -/* MUST eventually converge on CONFIG_NODE */ +/*
- MUST eventually converge either on CONFIG_NODE for every config node or
- on CONFIG_ENABLE for every other user defined node.
- */
The comment is wrong. There is no CONFIG_ENABLE node but the
You're right, it's ENABLE_NODE of course.
comment sounds dangerous too. The ENABLE_NODE might be password protected and I would like to avoid a situation where we come from a 'child' of the VIEW_NODE and end in 'ENABLE_NODE'.
The implementation cares about that (at least for the base nodes). It is not checked whether the go_parent callback does this and there isn't a way yet, to distinguish a view node from an enable node (since there is only is_config_child()/is_config_node() ).
The comment isn't quite explicit about that, but 'user defined node' was meant to refer to node (id's) above CONFIG_NODE, and there aren't any of these nodes that are neither config nor enable (yet).
OTOH, since that doesn't seem to be clear enough I'm going to reword it.
Jacob
This patch removes an assertion of node > CONFIG_NODE and changes the function to handle all nodes properly. For the sake of completeness, the generic 'exit' command implementation is extended to work properly with all nodes, too. --- src/vty/command.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/src/vty/command.c b/src/vty/command.c index 7f76ced..d6a2c18 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -29,7 +29,6 @@ Boston, MA 02111-1307, USA. */ #include <errno.h> #define _XOPEN_SOURCE #include <unistd.h> -#include <assert.h> #include <ctype.h> #include <time.h> #include <sys/time.h> @@ -1884,15 +1883,33 @@ char **cmd_complete_command(vector vline, struct vty *vty, int *status) }
/* return parent node */ -/* MUST eventually converge on CONFIG_NODE */ +/* + * This function MUST eventually converge on a node when called repeatedly, + * there must not be any cycles. + * All 'config' nodes shall converge on CONFIG_NODE. + * All other 'enable' nodes shall converge on ENABLE_NODE. + * All 'view' only nodes shall converge on VIEW_NODE. + * All other nodes shall converge on themselves or it must be ensured, + * that the user's rights are not extended anyhow by calling this function. + * + * Note that these requirements also apply to all functions that are used + * as go_parent_cb. + * Note also that this function relies on the is_config_child callback to + * recognize non-config nodes if go_parent_cb is not set. + */ enum node_type vty_go_parent(struct vty *vty) { - assert(vty->node > CONFIG_NODE); - switch (vty->node) { + case AUTH_NODE: + case VIEW_NODE: + case ENABLE_NODE: case CONFIG_NODE: break;
+ case AUTH_ENABLE_NODE: + vty->node = VIEW_NODE; + break; + case CFG_LOG_NODE: case VTY_NODE: vty->node = CONFIG_NODE; @@ -1901,8 +1918,10 @@ enum node_type vty_go_parent(struct vty *vty) default: if (host.app_info->go_parent_cb) host.app_info->go_parent_cb(vty); - else + else if (is_config_child(vty)) vty->node = CONFIG_NODE; + else + vty->node = VIEW_NODE; break; }
@@ -2267,6 +2286,7 @@ gDEFUN(config_exit, config_exit_cmd, "exit", "Exit current mode and down to previous mode\n") { switch (vty->node) { + case AUTH_NODE: case VIEW_NODE: case ENABLE_NODE: if (0) //vty_shell (vty))
-
Holger Hans Peter Freyther -
Jacob Erlbeck