10 Sep
2013
10 Sep
'13
9:07 a.m.
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) {
--
1.7.9.5
10 Sep
10 Sep
9:07 a.m.
New subject: [PATCH 2/2] vty: Allow vty_go_parent() in all nodes.
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))
--
1.7.9.5
11:06 a.m.
New subject: [PATCH 2/2] vty: Allow vty_go_parent() in all nodes.
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'.
1:31 p.m.
New subject: [PATCH 2/2] vty: Allow vty_go_parent() in all nodes.
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:
You're right, it's ENABLE_NODE of course.
/* 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'.
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
2:04 p.m.
New subject: [PATCH] vty: Allow vty_go_parent() in all nodes.
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))
--
1.7.9.5
3882
days inactive
3882
days old
4 comments
2 participants
participants (2)
-
Holger Hans Peter Freyther -
Jacob Erlbeck