lib: keep hash of node's commands to detect duplicate installs
* command.h: (struct cmd_node) Add a hash, so duplicate installs of
a cmd_element to a command node can be detected. To help catch
strays from the VIEW/ENABLE node consolidation particularly
(installs to VIEW automatically install to ENABLE too now).
* command.c: (cmd_hash_{key,cmp}) helpers for the hash - just directly
on the pointer value is sufficient to catch the main problem.
(install_node) setup the hash for the command node.
(install_element) check for duplicate installs.
The assert on the cmd_parse_format seems misplaced.
(install_default_basic) separate the basic, VIEW, node default commands
to here.
(cmd_init) get rid of dupes, given consolidation.
(cmd_terminate) clean up the node command hash.
Not done: The (struct cmd_node)'s vector could be replaced with the
cmd hash, however much of the command parser depends heavily on the
vector and it's a lot of work to change. A vector_lookup_value could
also work, particularly if vector could be backed by a hash.
The duplicate check could be disabled in releases - but useful in
development. It's a little extra overhead at startup. The command
initialisation overhead is already something that bites in
micro-benchmarks - makes it easy for other implementations to show
how much faster they are with benchmarks where other load is low
enough that startup time is a factor.
diff --git a/lib/command.c b/lib/command.c
index 12e8bb1..fa2ef11 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -212,6 +212,18 @@
return str;
}
+static unsigned int
+cmd_hash_key (void *p)
+{
+ return (uintptr_t) p;
+}
+
+static int
+cmd_hash_cmp (const void *a, const void *b)
+{
+ return a == b;
+}
+
/* Install top node of command vector. */
void
install_node (struct cmd_node *node,
@@ -220,6 +232,7 @@
vector_set_index (cmdvec, node->node, node);
node->func = func;
node->cmd_vector = vector_init (VECTOR_MIN_SIZE);
+ node->cmd_hash = hash_create (cmd_hash_key, cmd_hash_cmp);
}
/* Breaking up string into each command piece. I assume given
@@ -608,7 +621,11 @@
/* cmd_init hasn't been called */
if (!cmdvec)
- return;
+ {
+ fprintf (stderr, "%s called before cmd_init, breakage likely\n",
+ __func__);
+ return;
+ }
cnode = vector_slot (cmdvec, ntype);
@@ -618,7 +635,17 @@
ntype);
exit (1);
}
-
+
+ if (hash_lookup (cnode->cmd_hash, cmd) != NULL)
+ {
+ fprintf (stderr,
+ "Multiple command installs to node %d of command:\n%s\n",
+ ntype, cmd->string);
+ return;
+ }
+
+ assert (hash_get (cnode->cmd_hash, cmd, hash_alloc_intern));
+
vector_set (cnode->cmd_vector, cmd);
if (cmd->tokens == NULL)
cmd->tokens = cmd_parse_format(cmd->string, cmd->doc);
@@ -4114,15 +4141,30 @@
return host.config;
}
-void
-install_default (enum node_type node)
+static void
+install_default_basic (enum node_type node)
{
install_element (node, &config_exit_cmd);
install_element (node, &config_quit_cmd);
- install_element (node, &config_end_cmd);
install_element (node, &config_help_cmd);
install_element (node, &config_list_cmd);
+}
+/* Install common/default commands for a privileged node */
+void
+install_default (enum node_type node)
+{
+ /* VIEW_NODE is inited below, via install_default_basic, and
+ install_element's of commands to VIEW_NODE automatically are
+ also installed to ENABLE_NODE.
+
+ For all other nodes, we must ensure install_default_basic is
+ also called/
+ */
+ if (node != VIEW_NODE && node != ENABLE_NODE)
+ install_default_basic (node);
+
+ install_element (node, &config_end_cmd);
install_element (node, &config_write_terminal_cmd);
install_element (node, &config_write_file_cmd);
install_element (node, &config_write_memory_cmd);
@@ -4142,7 +4184,7 @@
/* Allocate initial top vector of commands. */
cmdvec = vector_init (VECTOR_MIN_SIZE);
-
+
/* Default host value settings. */
host.name = NULL;
host.password = NULL;
@@ -4165,10 +4207,8 @@
install_element (VIEW_NODE, &show_version_cmd);
if (terminal)
{
- install_element (VIEW_NODE, &config_list_cmd);
- install_element (VIEW_NODE, &config_exit_cmd);
- install_element (VIEW_NODE, &config_quit_cmd);
- install_element (VIEW_NODE, &config_help_cmd);
+ install_default_basic (VIEW_NODE);
+
install_element (VIEW_NODE, &config_enable_cmd);
install_element (VIEW_NODE, &config_terminal_length_cmd);
install_element (VIEW_NODE, &config_terminal_no_length_cmd);
@@ -4176,10 +4216,6 @@
install_element (VIEW_NODE, &show_commandtree_cmd);
install_element (VIEW_NODE, &echo_cmd);
- install_element (RESTRICTED_NODE, &config_list_cmd);
- install_element (RESTRICTED_NODE, &config_exit_cmd);
- install_element (RESTRICTED_NODE, &config_quit_cmd);
- install_element (RESTRICTED_NODE, &config_help_cmd);
install_element (RESTRICTED_NODE, &config_enable_cmd);
install_element (RESTRICTED_NODE, &config_terminal_length_cmd);
install_element (RESTRICTED_NODE, &config_terminal_no_length_cmd);
@@ -4195,15 +4231,9 @@
install_element (ENABLE_NODE, ©_runningconfig_startupconfig_cmd);
}
install_element (ENABLE_NODE, &show_startup_config_cmd);
- install_element (ENABLE_NODE, &show_version_cmd);
- install_element (ENABLE_NODE, &show_commandtree_cmd);
if (terminal)
{
- install_element (ENABLE_NODE, &config_terminal_length_cmd);
- install_element (ENABLE_NODE, &config_terminal_no_length_cmd);
- install_element (ENABLE_NODE, &show_logging_cmd);
- install_element (ENABLE_NODE, &echo_cmd);
install_element (ENABLE_NODE, &config_logmsg_cmd);
install_default (CONFIG_NODE);
@@ -4329,6 +4359,9 @@
cmd_terminate_element(cmd_element);
vector_free (cmd_node_v);
+ hash_clean (cmd_node->cmd_hash, NULL);
+ hash_free (cmd_node->cmd_hash);
+ cmd_node->cmd_hash = NULL;
}
vector_free (cmdvec);