[Xorp-hackers] Possible fix for XORP bug 603

[Justin Fletcher]

You may get an invalid <[Enter]> and | options in help, as in:

DUT-2> show ospf4 ?
Possible completions:
  <[Enter]>       Execute this command   <==***Should not be displayed***
  database        Show LSA database
  neighbor        Show Neighbors
  |               Pipe through this command  <==***Should not be displayed***

As these command fail with "ERROR: no matching command" if the command is executed, I've come up with possible fix which only displays these options if the command is valid, using the same mechanism as that which generated the error message in the first place.

In cli_client.cc, add the command validation:

--- cli_client.cc.bak   2006-04-25 02:13:13.000000000 +0000
+++ cli_client.cc       2006-04-25 02:16:25.000000000 +0000
@@ -35,6 +35,10 @@
 #include "cli_command_pipe.hh"
 #include "cli_private.hh"
 
+#include "rtrmgr/op_commands.hh"
+
+extern OpCommandList* ocl;
+
 #ifdef HOST_OS_WINDOWS
 #define isatty(x) (x).is_console()
 #endif
@@ -1315,6 +1319,7 @@
        word_end--;                     // XXX: exclude the '?' character
 
     bool can_execute = false, can_pipe = false;
+    bool cmd_valid = false;            // Is command valid?
     list<CliCommand *>::iterator iter;
     for (iter = curr_cli_command->child_command_list().begin();
         iter != curr_cli_command->child_command_list().end();
@@ -1326,13 +1331,25 @@
            is_found = true;
     }
     if (is_found) {
+
+       // Is command valid?
+
+       // Build temporary command with '?' stripped off
+
+       string command_name = line;
+       command_name.erase(word_end - 1, 2);
+       cout << "command_name: '" << command_name << "'" << endl;
+       if (ocl->op_command_valid(command_name)) {
+           cmd_valid = true;
+       }
+
        cli_print("\nPossible completions:\n");
-       if (can_execute) {
+       if (can_execute && cmd_valid) {
            cli_print(c_format("  %-15s %s\r\n",
                               "<[Enter]>", "Execute this command"));
        }
        cli_print(command_help_string);
-       if (can_pipe) {
+       if (can_pipe && cmd_valid) {
            cli_print(c_format("  %-15s %s\r\n",
                               "|", "Pipe through this command"));
        }

and add the validator functions to op_commands:

--- op_commands.hh.bak  2006-04-25 02:21:50.000000000 +0000
+++ op_commands.hh      2006-04-25 02:22:58.000000000 +0000
@@ -206,6 +206,7 @@
     void set_slave_config_tree(SlaveConfigTree* sct) { _slave_config_tree = sct; }
     bool check_variable_name(const string& variable_name) const;
     OpCommand* find_op_command(const list<string>& command_parts);
+    bool op_command_valid(const string& command_name);
     OpCommand* add_op_command(const OpCommand& op_command);
     bool command_match(const list<string>& command_parts,
                       bool exact_match) const;

--- op_commands.cc.bak  2006-04-25 02:20:43.000000000 +0000
+++ op_commands.cc      2006-04-25 02:20:16.000000000 +0000
@@ -696,6 +696,17 @@
 }
 
 bool
+OpCommandList::op_command_valid(const string& command_name)
+{
+    list<OpCommand*>::const_iterator iter;
+    for (iter = _op_commands.begin(); iter != _op_commands.end(); ++iter) {
+       if ((*iter)->command_name() == command_name)
+           return true;
+    }
+    return false;
+}
+
+bool
 OpCommandList::command_match(const list<string>& command_parts,
                             bool exact_match) const
 {

Finally, there's a global variable to tie the two together:

--- xorpsh_main.cc.bak  2006-04-25 02:25:47.000000000 +0000
+++ xorpsh_main.cc      2006-04-25 02:27:27.000000000 +0000
@@ -72,6 +72,7 @@
 static void signal_handler(int signal_value);
 static void exit_handler(CliClient*);
 
+OpCommandList *ocl;            // Global command list
 
 static void
 announce_waiting()
@@ -892,6 +893,7 @@
        string xname = "xorpsh" + c_format("-%d-%s", getpid(), hostname);
        XorpShell xorpsh(eventloop, xname, xorp_binary_root_dir(),
                         template_dir, xrl_targets_dir, verbose);
+       ocl = xorpsh.op_cmd_list();
        xorpsh.run(commands);
     } catch (const InitError& e) {
        XLOG_ERROR("xorpsh exiting due to an init error: %s", e.why().c_str());

What I don't like about this is that it requires a global variable to access the command list from the help function.  What I do like about this is that it works!

Suggestions? Alternatives? Just do it? :-)

Justin Fletcher
Vyatta