From 6432969db96ba03452b0e13906d85e718d6c633e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 16 Dec 2016 22:36:21 +0100 Subject: [PATCH 1/8] lib: parser: remove previous deduplication code Rewritten in next commit. Signed-off-by: David Lamparter --- lib/command_parse.y | 111 ++------------------------------------------ 1 file changed, 3 insertions(+), 108 deletions(-) diff --git a/lib/command_parse.y b/lib/command_parse.y index 1c220b6b69..0c415af3aa 100644 --- a/lib/command_parse.y +++ b/lib/command_parse.y @@ -120,15 +120,6 @@ static const char * doc_next (struct parser_ctx *ctx); - static struct graph_node * - node_adjacent (struct graph_node *, struct graph_node *); - - static struct graph_node * - add_edge_dedup (struct graph_node *, struct graph_node *); - - static int - cmp_token (struct cmd_token *, struct cmd_token *); - static struct graph_node * new_token_node (struct parser_ctx *, enum cmd_token_type type, @@ -179,14 +170,14 @@ start: } | cmd_token_seq placeholder_token '.' '.' '.' { - if ((ctx->currnode = add_edge_dedup (ctx->currnode, $2)) != $2) + if ((ctx->currnode = graph_add_edge (ctx->currnode, $2)) != $2) graph_delete_node (ctx->graph, $2); ((struct cmd_token *)ctx->currnode->data)->allowrepeat = 1; // adding a node as a child of itself accepts any number // of the same token, which is what we want for variadics - add_edge_dedup (ctx->currnode, ctx->currnode); + graph_add_edge (ctx->currnode, ctx->currnode); // tack on the command element terminate_graph (&@1, ctx, ctx->currnode); @@ -201,7 +192,7 @@ cmd_token_seq: cmd_token: simple_token { - if ((ctx->currnode = add_edge_dedup (ctx->currnode, $1)) != $1) + if ((ctx->currnode = graph_add_edge (ctx->currnode, $1)) != $1) graph_delete_node (ctx->graph, $1); } | selector @@ -418,9 +409,6 @@ terminate_graph (CMD_YYLTYPE *locp, struct parser_ctx *ctx, struct graph_node *end_element_node = graph_new_node (ctx->graph, element, NULL); - if (node_adjacent (finalnode, end_token_node)) - cmd_yyerror (locp, ctx, "Duplicate command."); - graph_add_edge (finalnode, end_token_node); graph_add_edge (end_token_node, end_element_node); } @@ -445,96 +433,3 @@ new_token_node (struct parser_ctx *ctx, enum cmd_token_type type, struct cmd_token *token = new_cmd_token (type, ctx->el->attr, text, doc); return graph_new_node (ctx->graph, token, (void (*)(void *)) &del_cmd_token); } - -/** - * Determines if there is an out edge from the first node to the second - */ -static struct graph_node * -node_adjacent (struct graph_node *first, struct graph_node *second) -{ - struct graph_node *adj; - for (unsigned int i = 0; i < vector_active (first->to); i++) - { - adj = vector_slot (first->to, i); - struct cmd_token *ftok = adj->data, - *stok = second->data; - if (cmp_token (ftok, stok)) - return adj; - } - return NULL; -} - -/** - * Creates an edge betwen two nodes, unless there is already an edge to an - * equivalent node. - * - * The first node's out edges are searched to see if any of them point to a - * node that is equivalent to the second node. If such a node exists, it is - * returned. Otherwise an edge is created from the first node to the second. - * - * @param from start node for edge - * @param to end node for edge - * @return the node which the new edge points to - */ -static struct graph_node * -add_edge_dedup (struct graph_node *from, struct graph_node *to) -{ - struct graph_node *existing = node_adjacent (from, to); - if (existing) - { - struct cmd_token *ex_tok = existing->data; - struct cmd_token *to_tok = to->data; - // NORMAL takes precedence over DEPRECATED takes precedence over HIDDEN - ex_tok->attr = (ex_tok->attr < to_tok->attr) ? ex_tok->attr : to_tok->attr; - return existing; - } - else - return graph_add_edge (from, to); -} - -/** - * Compares two cmd_token's for equality, - * - * As such, this function is the working definition of token equality - * for parsing purposes and determines overall graph structure. - */ -static int -cmp_token (struct cmd_token *first, struct cmd_token *second) -{ - // compare types - if (first->type != second->type) return 0; - - switch (first->type) { - case WORD_TKN: - case VARIABLE_TKN: - if (first->text && second->text) - { - if (strcmp (first->text, second->text)) - return 0; - } - else if (first->text != second->text) return 0; - break; - case RANGE_TKN: - if (first->min != second->min || first->max != second->max) - return 0; - break; - /* selectors and options should be equal if their subgraphs are equal, - * but the graph isomorphism problem is not known to be solvable in - * polynomial time so we consider selectors and options inequal in all - * cases; ultimately this forks the graph, but the matcher can handle - * this regardless - */ - case FORK_TKN: - return 0; - - /* end nodes are always considered equal, since each node may only - * have one END_TKN child at a time - */ - case START_TKN: - case END_TKN: - case JOIN_TKN: - default: - break; - } - return 1; -} From de8f7a39836a8f121370c1506cc00e3321f284a2 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 16 Dec 2016 23:27:39 +0100 Subject: [PATCH 2/8] lib: parser: rewrite token deduplication Merge the parsed graph into the existing one as a separate step. This makes it possible to merge identical subgraphs, which is used e.g. in bgpd for neighbor names. Signed-off-by: David Lamparter --- lib/command.c | 321 +++++++++++++++++++++++++++++++++++++++++- lib/command.h | 6 + lib/grammar_sandbox.c | 7 +- 3 files changed, 332 insertions(+), 2 deletions(-) diff --git a/lib/command.c b/lib/command.c index 4a84386858..af6e515ce3 100644 --- a/lib/command.c +++ b/lib/command.c @@ -304,6 +304,272 @@ cmd_prompt (enum node_type node) return cnode->prompt; } +static bool +cmd_nodes_link (struct graph_node *from, struct graph_node *to) +{ + for (size_t i = 0; i < vector_active (from->to); i++) + if (vector_slot (from->to, i) == to) + return true; + return false; +} + +static bool cmd_nodes_equal (struct graph_node *ga, struct graph_node *gb); + +/* returns a single node to be excluded as "next" from iteration + * - for JOIN_TKN, never continue back to the FORK_TKN + * - in all other cases, don't try the node itself (in case of "...") + */ +static inline struct graph_node * +cmd_loopstop(struct graph_node *gn) +{ + struct cmd_token *tok = gn->data; + if (tok->type == JOIN_TKN) + return tok->forkjoin; + else + return gn; +} + +static bool +cmd_subgraph_equal (struct graph_node *ga, struct graph_node *gb, + struct graph_node *a_join) +{ + size_t i, j; + struct graph_node *a_fork, *b_fork; + a_fork = cmd_loopstop (ga); + b_fork = cmd_loopstop (gb); + + if (vector_active (ga->to) != vector_active (gb->to)) + return false; + for (i = 0; i < vector_active (ga->to); i++) + { + struct graph_node *cga = vector_slot (ga->to, i); + + for (j = 0; j < vector_active (gb->to); j++) + { + struct graph_node *cgb = vector_slot (gb->to, i); + + if (cga == a_fork && cgb != b_fork) + continue; + if (cga == a_fork && cgb == b_fork) + break; + + if (cmd_nodes_equal (cga, cgb)) + { + if (cga == a_join) + break; + if (cmd_subgraph_equal (cga, cgb, a_join)) + break; + } + } + if (j == vector_active (gb->to)) + return false; + } + return true; +} + +/* deep compare -- for FORK_TKN, the entire subgraph is compared. + * this is what's needed since we're not currently trying to partially + * merge subgraphs */ +static bool +cmd_nodes_equal (struct graph_node *ga, struct graph_node *gb) +{ + struct cmd_token *a = ga->data, *b = gb->data; + + if (a->type != b->type || a->allowrepeat != b->allowrepeat) + return false; + if (a->type < SPECIAL_TKN && strcmp (a->text, b->text)) + return false; + /* one a ..., the other not. */ + if (cmd_nodes_link (ga, ga) != cmd_nodes_link (gb, gb)) + return false; + + switch (a->type) + { + case RANGE_TKN: + return a->min == b->min && a->max == b->max; + + case FORK_TKN: + /* one is keywords, the other just option or selector ... */ + if (cmd_nodes_link (a->forkjoin, ga) != cmd_nodes_link (b->forkjoin, gb)) + return false; + if (cmd_nodes_link (ga, a->forkjoin) != cmd_nodes_link (gb, b->forkjoin)) + return false; + return cmd_subgraph_equal (ga, gb, a->forkjoin); + + default: + return true; + } +} + +static void +cmd_fork_bump_attr (struct graph_node *gn, struct graph_node *join, + u_char attr) +{ + size_t i; + struct cmd_token *tok = gn->data; + struct graph_node *stop = cmd_loopstop (gn); + + tok->attr = attr; + for (i = 0; i < vector_active (gn->to); i++) + { + struct graph_node *next = vector_slot (gn->to, i); + if (next == stop || next == join) + continue; + cmd_fork_bump_attr (next, join, attr); + } +} + +/* move an entire subtree from the temporary graph resulting from + * parse() into the permanent graph for the command node. + * + * this touches rather deeply into the graph code unfortunately. + */ +static void +cmd_reparent_tree (struct graph *fromgraph, struct graph *tograph, + struct graph_node *node) +{ + struct graph_node *stop = cmd_loopstop (node); + size_t i; + + for (i = 0; i < vector_active (fromgraph->nodes); i++) + if (vector_slot (fromgraph->nodes, i) == node) + { + /* agressive iteration punching through subgraphs - may hit some + * nodes twice. reparent only if found on old graph */ + vector_unset (fromgraph->nodes, i); + vector_set (tograph->nodes, node); + break; + } + + for (i = 0; i < vector_active (node->to); i++) + { + struct graph_node *next = vector_slot (node->to, i); + if (next != stop) + cmd_reparent_tree (fromgraph, tograph, next); + } +} + +static void +cmd_free_recur (struct graph *graph, struct graph_node *node, + struct graph_node *stop) +{ + struct graph_node *next, *nstop; + + for (size_t i = vector_active (node->to); i; i--) + { + next = vector_slot (node->to, i - 1); + if (next == stop) + continue; + nstop = cmd_loopstop (next); + if (nstop != next) + cmd_free_recur (graph, next, nstop); + cmd_free_recur (graph, nstop, stop); + } + graph_delete_node (graph, node); +} + +static void +cmd_free_node (struct graph *graph, struct graph_node *node) +{ + struct cmd_token *tok = node->data; + if (tok->type == JOIN_TKN) + cmd_free_recur (graph, tok->forkjoin, node); + graph_delete_node (graph, node); +} + +/* recursive graph merge. call with + * old ~= new + * (which holds true for old == START_TKN, new == START_TKN) + */ +static void +cmd_merge_nodes (struct graph *oldgraph, struct graph *newgraph, + struct graph_node *old, struct graph_node *new, + int direction) +{ + struct cmd_token *tok; + struct graph_node *old_skip, *new_skip; + old_skip = cmd_loopstop (old); + new_skip = cmd_loopstop (new); + + assert (direction == 1 || direction == -1); + + tok = old->data; + tok->refcnt += direction; + + size_t j, i; + for (j = 0; j < vector_active (new->to); j++) + { + struct graph_node *cnew = vector_slot (new->to, j); + if (cnew == new_skip) + continue; + + for (i = 0; i < vector_active (old->to); i++) + { + struct graph_node *cold = vector_slot (old->to, i); + if (cold == old_skip) + continue; + + if (cmd_nodes_equal (cold, cnew)) + { + struct cmd_token *told = cold->data, *tnew = cnew->data; + + if (told->type == END_TKN) + { + if (direction < 0) + { + graph_delete_node (oldgraph, vector_slot (cold->to, 0)); + graph_delete_node (oldgraph, cold); + } + else + /* force no-match handling to install END_TKN */ + i = vector_active (old->to); + break; + } + + /* the entire fork compared as equal, we continue after it. */ + if (told->type == FORK_TKN) + { + if (tnew->attr < told->attr && direction > 0) + cmd_fork_bump_attr (cold, told->forkjoin, tnew->attr); + /* XXX: no reverse bump on uninstall */ + told = (cold = told->forkjoin)->data; + tnew = (cnew = tnew->forkjoin)->data; + } + if (tnew->attr < told->attr) + told->attr = tnew->attr; + + cmd_merge_nodes (oldgraph, newgraph, cold, cnew, direction); + break; + } + } + /* nothing found => add new to old */ + if (i == vector_active (old->to) && direction > 0) + { + assert (vector_count (cnew->from) == + cmd_nodes_link (cnew, cnew) ? 2 : 1); + graph_remove_edge (new, cnew); + + cmd_reparent_tree (newgraph, oldgraph, cnew); + + graph_add_edge (old, cnew); + } + } + + if (!tok->refcnt) + cmd_free_node (oldgraph, old); +} + +void +cmd_merge_graphs (struct graph *old, struct graph *new, int direction) +{ + assert (vector_active (old->nodes) >= 1); + assert (vector_active (new->nodes) >= 1); + + cmd_merge_nodes (old, new, + vector_slot (old->nodes, 0), vector_slot (new->nodes, 0), + direction); +} + /* Install a command into a node. */ void install_element (enum node_type ntype, struct cmd_element *cmd) @@ -337,13 +603,65 @@ install_element (enum node_type ntype, struct cmd_element *cmd) assert (hash_get (cnode->cmd_hash, cmd, hash_alloc_intern)); - command_parse_format (cnode->cmdgraph, cmd); + struct graph *graph = graph_new(); + struct cmd_token *token = new_cmd_token (START_TKN, CMD_ATTR_NORMAL, NULL, NULL); + graph_new_node (graph, token, (void (*)(void *)) &del_cmd_token); + + command_parse_format (graph, cmd); + cmd_merge_graphs (cnode->cmdgraph, graph, +1); + graph_delete_graph (graph); + vector_set (cnode->cmd_vector, cmd); if (ntype == VIEW_NODE) install_element (ENABLE_NODE, cmd); } +void +uninstall_element (enum node_type ntype, struct cmd_element *cmd) +{ + struct cmd_node *cnode; + + /* cmd_init hasn't been called */ + if (!cmdvec) + { + fprintf (stderr, "%s called before cmd_init, breakage likely\n", + __func__); + return; + } + + cnode = vector_slot (cmdvec, ntype); + + if (cnode == NULL) + { + fprintf (stderr, "Command node %d doesn't exist, please check it\n", + ntype); + exit (EXIT_FAILURE); + } + + if (hash_release (cnode->cmd_hash, cmd) == NULL) + { + fprintf (stderr, + "Trying to uninstall non-installed command (node %d):\n%s\n", + ntype, cmd->string); + return; + } + + vector_unset_value (cnode->cmd_vector, cmd); + + struct graph *graph = graph_new(); + struct cmd_token *token = new_cmd_token (START_TKN, CMD_ATTR_NORMAL, NULL, NULL); + graph_new_node (graph, token, (void (*)(void *)) &del_cmd_token); + + command_parse_format (graph, cmd); + cmd_merge_graphs (cnode->cmdgraph, graph, -1); + graph_delete_graph (graph); + + if (ntype == VIEW_NODE) + uninstall_element (ENABLE_NODE, cmd); +} + + static const unsigned char itoa64[] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; @@ -2417,6 +2735,7 @@ new_cmd_token (enum cmd_token_type type, u_char attr, token->attr = attr; token->text = text ? XSTRDUP (MTYPE_CMD_TEXT, text) : NULL; token->desc = desc ? XSTRDUP (MTYPE_CMD_DESC, desc) : NULL; + token->refcnt = 1; token->arg = NULL; token->allowrepeat = false; diff --git a/lib/command.h b/lib/command.h index 04bba9e412..7986c676fc 100644 --- a/lib/command.h +++ b/lib/command.h @@ -200,6 +200,7 @@ struct cmd_token enum cmd_token_type type; // token type u_char attr; // token attributes bool allowrepeat; // matcher allowed to match token repetively? + uint32_t refcnt; char *text; // token text char *desc; // token description @@ -402,6 +403,10 @@ extern void install_node (struct cmd_node *, int (*) (struct vty *)); extern void install_default (enum node_type); extern void install_element (enum node_type, struct cmd_element *); +/* known issue with uninstall_element: changes to cmd_token->attr (i.e. + * deprecated/hidden) are not reversed. */ +extern void uninstall_element (enum node_type, struct cmd_element *); + /* Concatenates argv[shift] through argv[argc-1] into a single NUL-terminated string with a space between each element (allocated using XMALLOC(MTYPE_TMP)). Returns NULL if shift >= argc. */ @@ -435,6 +440,7 @@ extern void del_cmd_token (struct cmd_token *); extern struct cmd_token *copy_cmd_token (struct cmd_token *); extern vector completions_to_vec (struct list *completions); +extern void cmd_merge_graphs (struct graph *old, struct graph *new, int direction); extern void command_parse_format (struct graph *graph, struct cmd_element *cmd); /* Export typical functions. */ diff --git a/lib/grammar_sandbox.c b/lib/grammar_sandbox.c index 315bd4d59c..0779f23060 100644 --- a/lib/grammar_sandbox.c +++ b/lib/grammar_sandbox.c @@ -68,7 +68,12 @@ DEFUN (grammar_test, cmd->func = NULL; // parse the command and install it into the command graph - command_parse_format (nodegraph, cmd); + struct graph *graph = graph_new(); + struct cmd_token *token = new_cmd_token (START_TKN, CMD_ATTR_NORMAL, NULL, NULL); + graph_new_node (graph, token, (void (*)(void *)) &del_cmd_token); + + command_parse_format (graph, cmd); + cmd_merge_graphs (nodegraph, graph, +1); return CMD_SUCCESS; } From fdc3d1ab837a81d27ab10d0bf65f349263870154 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 25 Jan 2017 04:14:07 +0100 Subject: [PATCH 3/8] tests: add uninstall_element in testcli Test uninstall_element(). The commandline-specified counter allows isolating memleaks more closely, differentiating one-off vs. repeated leaks. Signed-off-by: David Lamparter --- tests/lib/cli/common_cli.c | 2 +- tests/lib/cli/common_cli.h | 2 +- tests/lib/cli/test_cli.c | 12 +++++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/lib/cli/common_cli.c b/tests/lib/cli/common_cli.c index 83196e04aa..104352f516 100644 --- a/tests/lib/cli/common_cli.c +++ b/tests/lib/cli/common_cli.c @@ -83,7 +83,7 @@ main (int argc, char **argv) vty_init (master); memory_init (); - test_init (); + test_init (argc, argv); vty_stdio (vty_do_exit); diff --git a/tests/lib/cli/common_cli.h b/tests/lib/cli/common_cli.h index 9c72b08e44..9e7fe99830 100644 --- a/tests/lib/cli/common_cli.h +++ b/tests/lib/cli/common_cli.h @@ -28,7 +28,7 @@ #include "command.h" /* function to be implemented by test */ -extern void test_init (void); +extern void test_init (int argc, char **argv); /* functions provided by common cli * (includes main()) diff --git a/tests/lib/cli/test_cli.c b/tests/lib/cli/test_cli.c index 1a316022e2..54b34bc799 100644 --- a/tests/lib/cli/test_cli.c +++ b/tests/lib/cli/test_cli.c @@ -38,8 +38,10 @@ DUMMY_DEFUN(cmd11, "alt a WORD"); DUMMY_DEFUN(cmd12, "alt a A.B.C.D"); DUMMY_DEFUN(cmd13, "alt a X:X::X:X"); -void test_init(void) +void test_init(int argc, char **argv) { + size_t repeat = argc > 1 ? strtoul(argv[1], NULL, 0) : 223; + install_element (ENABLE_NODE, &cmd0_cmd); install_element (ENABLE_NODE, &cmd1_cmd); install_element (ENABLE_NODE, &cmd2_cmd); @@ -53,4 +55,12 @@ void test_init(void) install_element (ENABLE_NODE, &cmd11_cmd); install_element (ENABLE_NODE, &cmd12_cmd); install_element (ENABLE_NODE, &cmd13_cmd); + for (size_t i = 0; i < repeat; i++) { + uninstall_element (ENABLE_NODE, &cmd5_cmd); + install_element (ENABLE_NODE, &cmd5_cmd); + } + for (size_t i = 0; i < repeat; i++) { + uninstall_element (ENABLE_NODE, &cmd13_cmd); + install_element (ENABLE_NODE, &cmd13_cmd); + } } From db85c8b38418cf28fb7acacb6ad59614817db5e5 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Feb 2017 16:42:49 +0100 Subject: [PATCH 4/8] lib: track name of command in cmd_element Signed-off-by: David Lamparter --- lib/command.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/command.h b/lib/command.h index 7986c676fc..29283418ce 100644 --- a/lib/command.h +++ b/lib/command.h @@ -220,6 +220,8 @@ struct cmd_element /* handler function for command */ int (*func) (const struct cmd_element *, struct vty *, int, struct cmd_token *[]); + + const char *name; /* symbol name for debugging */ }; /* Return value of the commands. */ @@ -252,6 +254,7 @@ struct cmd_element .doc = helpstr, \ .attr = attrs, \ .daemon = dnum, \ + .name = #cmdname, \ }; #define DEFUN_CMD_FUNC_DECL(funcname) \ From 0a22b97922ed61415ae9f57e57741d4af3ddb217 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Feb 2017 16:43:17 +0100 Subject: [PATCH 5/8] grammar_sandbox: add ambiguous-command finder Best run in vtysh. Signed-off-by: David Lamparter --- lib/grammar_sandbox.c | 149 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 2 deletions(-) diff --git a/lib/grammar_sandbox.c b/lib/grammar_sandbox.c index 0779f23060..56733b2ecd 100644 --- a/lib/grammar_sandbox.c +++ b/lib/grammar_sandbox.c @@ -27,6 +27,7 @@ #include "command.h" #include "memory_vty.h" #include "graph.h" +#include "linklist.h" #include "command_match.h" #define GRAMMAR_STR "CLI grammar sandbox\n" @@ -270,6 +271,151 @@ DEFUN (grammar_test_dot, return CMD_SUCCESS; } +struct cmd_permute_item +{ + char *cmd; + struct cmd_element *el; +}; + +static void +cmd_permute_free (void *arg) +{ + struct cmd_permute_item *i = arg; + XFREE (MTYPE_TMP, i->cmd); + XFREE (MTYPE_TMP, i); +} + +static int +cmd_permute_cmp (void *a, void *b) +{ + struct cmd_permute_item *aa = a, *bb = b; + return strcmp (aa->cmd, bb->cmd); +} + +static void +cmd_graph_permute (struct list *out, struct graph_node **stack, + size_t stackpos, char *cmd) +{ + struct graph_node *gn = stack[stackpos]; + struct cmd_token *tok = gn->data; + char *appendp = cmd + strlen(cmd); + size_t i, j; + + if (tok->type < SPECIAL_TKN) + { + sprintf (appendp, "%s ", tok->text); + appendp += strlen (appendp); + } + else if (tok->type == END_TKN) + { + struct cmd_permute_item *i = XMALLOC (MTYPE_TMP, sizeof (*i)); + i->el = ((struct graph_node *)vector_slot (gn->to, 0))->data; + i->cmd = XSTRDUP (MTYPE_TMP, cmd); + i->cmd[strlen(cmd) - 1] = '\0'; + listnode_add_sort (out, i); + return; + } + + if (++stackpos == MAXDEPTH) + return; + + for (i = 0; i < vector_active (gn->to); i++) + { + struct graph_node *gnext = vector_slot (gn->to, i); + for (j = 0; j < stackpos; j++) + if (stack[j] == gnext) + break; + if (j != stackpos) + continue; + + stack[stackpos] = gnext; + *appendp = '\0'; + cmd_graph_permute (out, stack, stackpos, cmd); + } +} + +static struct list * +cmd_graph_permutations (struct graph *graph) +{ + char accumulate[2048] = ""; + struct graph_node *stack[MAXDEPTH]; + + struct list *rv = list_new (); + rv->cmp = cmd_permute_cmp; + rv->del = cmd_permute_free; + stack[0] = vector_slot (graph->nodes, 0); + cmd_graph_permute (rv, stack, 0, accumulate); + return rv; +} + +extern vector cmdvec; + +DEFUN (grammar_findambig, + grammar_findambig_cmd, + "grammar find-ambiguous [{printall|nodescan}]", + GRAMMAR_STR + "Find ambiguous commands\n" + "Print all permutations\n" + "Scan all nodes\n") +{ + struct list *commands; + struct cmd_permute_item *prev = NULL, *cur = NULL; + struct listnode *ln; + int i, printall, scan, scannode = 0; + + i = 0; + printall = argv_find (argv, argc, "printall", &i); + i = 0; + scan = argv_find (argv, argc, "nodescan", &i); + + if (scan && nodegraph_free) + { + graph_delete_graph (nodegraph_free); + nodegraph_free = NULL; + } + + if (!scan && !nodegraph) + { + vty_out(vty, "nodegraph uninitialized\r\n"); + return CMD_WARNING; + } + + do { + if (scan) + { + struct cmd_node *cnode = vector_slot (cmdvec, scannode++); + if (!cnode) + continue; + nodegraph = cnode->cmdgraph; + if (!nodegraph) + continue; + vty_out (vty, "scanning node %d%s", scannode - 1, VTY_NEWLINE); + } + + commands = cmd_graph_permutations (nodegraph); + prev = NULL; + for (ALL_LIST_ELEMENTS_RO (commands, ln, cur)) + { + int same = prev && !strcmp (prev->cmd, cur->cmd); + if (printall && !same) + vty_out (vty, "'%s'%s", cur->cmd, VTY_NEWLINE); + if (same) + { + vty_out (vty, "'%s' AMBIGUOUS:%s", cur->cmd, VTY_NEWLINE); + vty_out (vty, " %s%s '%s'%s", prev->el->name, VTY_NEWLINE, prev->el->string, VTY_NEWLINE); + vty_out (vty, " %s%s '%s'%s", cur->el->name, VTY_NEWLINE, cur->el->string, VTY_NEWLINE); + vty_out (vty, "%s", VTY_NEWLINE); + } + prev = cur; + } + list_delete (commands); + } while (scan && scannode < LINK_PARAMS_NODE); + + if (scan) + nodegraph = NULL; + return CMD_SUCCESS; +} + DEFUN (grammar_init_graph, grammar_init_graph_cmd, "grammar init", @@ -284,8 +430,6 @@ DEFUN (grammar_init_graph, return CMD_SUCCESS; } -extern vector cmdvec; - DEFUN (grammar_access, grammar_access_cmd, "grammar access (0-65535)", @@ -322,6 +466,7 @@ void grammar_sandbox_init(void) { install_element (ENABLE_NODE, &grammar_test_match_cmd); install_element (ENABLE_NODE, &grammar_test_complete_cmd); install_element (ENABLE_NODE, &grammar_test_doc_cmd); + install_element (ENABLE_NODE, &grammar_findambig_cmd); install_element (ENABLE_NODE, &grammar_init_graph_cmd); install_element (ENABLE_NODE, &grammar_access_cmd); } From dab8cd005fae1fa33ef7203e85dd7ab93108f418 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Feb 2017 16:56:10 +0100 Subject: [PATCH 6/8] *: fix ambiguous commands Some "show [ip] bgp ipv4 encap ..." commands remaining ambiguous. Signed-off-by: David Lamparter --- bgpd/bgp_vty.c | 2 +- ospf6d/ospf6_asbr.c | 8 ++++---- pimd/pim_cmd.c | 2 +- zebra/zebra_mpls_vty.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 01d1768691..b4425297b3 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -2841,7 +2841,7 @@ DEFUN (neighbor_peer_group, DEFUN (no_neighbor, no_neighbor_cmd, - "no neighbor [remote-as <(1-4294967295)|internal|external>]", + "no neighbor [remote-as <(1-4294967295)|internal|external>]>", NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 diff --git a/ospf6d/ospf6_asbr.c b/ospf6d/ospf6_asbr.c index 643519e3fa..5f2d662b78 100644 --- a/ospf6d/ospf6_asbr.c +++ b/ospf6d/ospf6_asbr.c @@ -1114,7 +1114,7 @@ DEFUN (ospf6_routemap_set_metric_type, /* delete "set metric-type" */ DEFUN (ospf6_routemap_no_set_metric_type, ospf6_routemap_no_set_metric_type_cmd, - "no set metric-type ", + "no set metric-type []", NO_STR "Set value\n" "Type of metric\n" @@ -1122,9 +1122,9 @@ DEFUN (ospf6_routemap_no_set_metric_type, "OSPF6 external type 2 metric\n") { VTY_DECLVAR_CONTEXT(route_map_index, route_map_index); - int idx_external = 3; - int ret = route_map_delete_set (route_map_index, - "metric-type", argv[idx_external]->arg); + char *ext = (argc == 4) ? argv[3]->text : NULL; + int ret = route_map_delete_set (route_map_index, + "metric-type", ext); return route_map_command_status (vty, ret); } diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index ae23499d97..62d8ad8e07 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -5964,7 +5964,7 @@ ip_msdp_show_sa_sg(struct vty *vty, const char *src, const char *grp, u_char uj) DEFUN (show_ip_msdp_sa_sg, show_ip_msdp_sa_sg_cmd, - "show ip msdp sa [A.B.C.D] [A.B.C.D] [json]", + "show ip msdp sa [A.B.C.D [A.B.C.D]] [json]", SHOW_STR IP_STR MSDP_STR diff --git a/zebra/zebra_mpls_vty.c b/zebra/zebra_mpls_vty.c index fcd0fff32c..dd381723c5 100644 --- a/zebra/zebra_mpls_vty.c +++ b/zebra/zebra_mpls_vty.c @@ -472,7 +472,7 @@ DEFUN (no_ip_route_tag_distance_label, DEFUN (no_ip_route_mask_distance_label, no_ip_route_mask_distance_label_cmd, - "no ip route A.B.C.D A.B.C.D (1-255)", + "no ip route A.B.C.D A.B.C.D (1-255) label WORD", NO_STR IP_STR "Establish static routes\n" @@ -486,7 +486,7 @@ DEFUN (no_ip_route_mask_distance_label, "One or more labels separated by '/'\n") { return zebra_static_ipv4 (vty, SAFI_UNICAST, 0, argv[3]->arg, argv[4]->arg, argv[5]->arg, NULL, NULL, - argv[6]->arg, NULL, NULL); + argv[6]->arg, NULL, argv[8]->arg); } DEFUN (no_ip_route_mask_tag_distance_label, From 37f9f36ec9b150474fe2347b79ccde2f41e1ec59 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Feb 2017 17:14:50 +0100 Subject: [PATCH 7/8] grammar_sandbox: count ambiguous commands ... and make the return value useful (for vtysh) Signed-off-by: David Lamparter --- lib/grammar_sandbox.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/grammar_sandbox.c b/lib/grammar_sandbox.c index 56733b2ecd..92292fced8 100644 --- a/lib/grammar_sandbox.c +++ b/lib/grammar_sandbox.c @@ -362,6 +362,7 @@ DEFUN (grammar_findambig, struct cmd_permute_item *prev = NULL, *cur = NULL; struct listnode *ln; int i, printall, scan, scannode = 0; + int ambig = 0; i = 0; printall = argv_find (argv, argc, "printall", &i); @@ -405,15 +406,20 @@ DEFUN (grammar_findambig, vty_out (vty, " %s%s '%s'%s", prev->el->name, VTY_NEWLINE, prev->el->string, VTY_NEWLINE); vty_out (vty, " %s%s '%s'%s", cur->el->name, VTY_NEWLINE, cur->el->string, VTY_NEWLINE); vty_out (vty, "%s", VTY_NEWLINE); + ambig++; } prev = cur; } list_delete (commands); + + vty_out (vty, "%s", VTY_NEWLINE); } while (scan && scannode < LINK_PARAMS_NODE); + vty_out (vty, "%d ambiguous commands found.%s", ambig, VTY_NEWLINE); + if (scan) nodegraph = NULL; - return CMD_SUCCESS; + return ambig == 0 ? CMD_SUCCESS : CMD_WARNING; } DEFUN (grammar_init_graph, From cd2726408a423e8b977f5906cc2aaee86c3d8327 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 10 Feb 2017 17:15:36 +0100 Subject: [PATCH 8/8] vtysh: make -c useful with -C (dryrun) -c was previously ignored when -C (dryrun/config-check) was present. Change so that -C -c creates an useful dry-run mode. Signed-off-by: David Lamparter --- vtysh/vtysh_main.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index 49aedae322..561eda0454 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -274,7 +274,7 @@ main (int argc, char **argv, char **env) const char *inputfile = NULL; const char *vtysh_configfile_name; struct cmd_rec { - const char *line; + char *line; struct cmd_rec *next; } *cmd = NULL; struct cmd_rec *tail = NULL; @@ -444,7 +444,7 @@ main (int argc, char **argv, char **env) } /* Start execution only if not in dry-run mode */ - if(dryrun) + if (dryrun && !cmd) { if (inputfile) { @@ -454,6 +454,39 @@ main (int argc, char **argv, char **env) { ret = vtysh_read_config(quagga_config_default); } + + exit(ret); + } + + if (dryrun && cmd) + { + vtysh_execute ("enable"); + while (cmd) + { + struct cmd_rec *cr; + char *cmdnow = cmd->line, *next; + do + { + next = strchr(cmdnow, '\n'); + if (next) + *next++ = '\0'; + + if (echo_command) + printf("%s%s\n", vtysh_prompt(), cmdnow); + + ret = vtysh_execute_no_pager(cmdnow); + if (!no_error && + ! (ret == CMD_SUCCESS || + ret == CMD_SUCCESS_DAEMON || + ret == CMD_WARNING)) + exit(1); + } + while ((cmdnow = next) != NULL); + + cr = cmd; + cmd = cmd->next; + XFREE(MTYPE_TMP, cr); + } exit(ret); }