On 08/31/2013 03:47 PM, Holger Hans Peter Freyther wrote:
On Fri, Aug 30, 2013 at 06:34:00PM +0200, Jacob Erlbeck wrote:
Note that in config mode if the tree is searched along the nodes toward the config node and a command is not found this way, a rollback is done by just replacing the vty's node and index member variable by the saved old values which might break the whole thing, when there has been a free() on the way.
okay, but in this case the config parsing will fail too. So right now we should be safe. Thanks for looking into this! o
The problem would rather arise in interactive mode when somebody mistypes a command. It don't know, why outer context searching is enabled in config mode only and if somebody someday would like to provide this in non-config sub-nodes, too. Even if not, the scheme used for UM2K (alloc() in the node command and free() in vty_go_parent()) must not be used for config nodes. Having the same function (vty_go_parent() ) for just looking up the outer nodes and unwinding the node contexts effectively forbids side-effects. Perhaps one could add an additional boolean parameter to select between both modes of operation and separate the search phase from the unwinding phase.
@@ -186,36 +130,24 @@ gDEFUN(ournode_exit, gDEFUN(ournode_end, ournode_end_cmd, "end", "End current mode and change to enable mode.") {
- if (vty->node >= CONFIG_NODE) {
this would be true for the OM2K node, which is not a configure node. Can you elaborate why not using the 'is config node' predicate is not necessary?
The 'end' command only makes sense for node >= ENABLE_NODE. If must not be applied before the vty is in enabled mode, so it's explicitly a no-op in these cases.
What I'm not sure about is whether the 'end' command should be available in the non-config nodes, too (or what's the pure cisco way of it). It doesn't 'end' the configuration then but it returns to the top node in a defined way, which might be helpful in automated scripting.
def _testConfigNetworkTree(self): self.vty.enable()
self.vty.verify("configure terminal",[''])
self.assertTrue(self.vty.verify("configure terminal",['']))where is the change from self.vty.verify to self assertTrue coming from? is this related to your change?
Not at all, it accidently went into that patch. I'd rather put it into the PATCH 2/6 'test' patch.