lib: Refactor format parser

Also fix a bug where selector elements beginning
with the same word are only parsed once

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/lib/command_graph.c b/lib/command_graph.c
index 07cc306505206d34fe3071d1b7c9480085c8f960..05570d5c7365833e6e7ea06fa26d974251dd270b 100644 (file)
--- a/lib/command_graph.c
+++ b/lib/command_graph.c
@@ -13,14 +13,6 @@
 struct graph_node *
 add_node(struct graph_node *parent, struct graph_node *child)
 {
-  struct graph_node *p_child;
-
-  for (unsigned int i = 0; i < vector_active(parent->children); i++)
-  {
-    p_child = vector_slot(parent->children, i);
-    if (cmp_node(child, p_child))
-      return p_child;
-  }
   vector_set(parent->children, child);
   child->refs++;
   return child;

diff --git a/lib/command_graph.h b/lib/command_graph.h
index 32a07ce4eae27970268d3993c495e8941c6547b8..35a0eee273e82b5341c356014ee69c3a0295382f 100644 (file)
--- a/lib/command_graph.h
+++ b/lib/command_graph.h
@@ -42,13 +42,10 @@ struct graph_node
 
 /*
  * Adds a node as a child of another node.
- * If the new parent has a child that is equal to the prospective child, as
- * determined by cmp_node, then a pointer to the existing node is returned and
- * the prospective child is not added. Otherwise the child node is returned.
  *
  * @param[in] parent node
  * @param[in] child node
- * @return pointer to child if it is added, pointer to existing child otherwise
+ * @return child node, for convenience
  */
 struct graph_node *
 add_node(struct graph_node *, struct graph_node *);

diff --git a/lib/command_parse.y b/lib/command_parse.y
index 57a0ba3f82b93bd655f614205e668f98e3626448..0c36c2a6dc92698ff11ebeff627ea13161109973 100644 (file)
--- a/lib/command_parse.y
+++ b/lib/command_parse.y
@@ -10,7 +10,11 @@
 
 %{
 extern int yylex(void);
-extern void yyerror(const char *);
+void yyerror(const char *);
+struct graph_node *
+node_exists(struct graph_node *, struct graph_node *);
+struct graph_node *
+node_replace(struct graph_node *, struct graph_node *);
 
 // compile with debugging facilities
 #define YYDEBUG 1
@@ -86,34 +90,35 @@ start:
   end->element = command;
 
   // add node
-  if (add_node(currnode, end) != end)
+  if (node_exists(currnode, end))
   {
     yyerror("Duplicate command.");
     YYABORT;
   }
+  add_node(currnode, end);
   fprintf(stderr, "Parsed full command successfully.\n");
 }
 | sentence_root cmd_token_seq '.' placeholder_token
 {
-  currnode = add_node(currnode, $4);
-  if (currnode != $4)
+  if ((currnode = node_replace(currnode, $4)) != $4)
     free_node ($4);
 
   // since varargs may match any number of the last token,
   // simply add this node as a child of itself and proceed
   // wth normal command termination procedure
-  add_node(currnode, currnode);
+  node_replace(currnode, currnode);
 
   // create leaf node
   struct graph_node *end = new_node(END_GN);
   end->element = command;
 
   // add node
-  if (add_node(currnode, end) != end)
+  if (node_exists(currnode, end))
   {
     yyerror("Duplicate command.");
     YYABORT;
   }
+  add_node(currnode, end);
   fprintf(stderr, "Parsed full command successfully.\n");
 }
 
@@ -122,8 +127,7 @@ sentence_root: WORD
   struct graph_node *root = new_node(WORD_GN);
   root->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
 
-  currnode = add_node(startnode, root);
-  if (currnode != root)
+  if ((currnode = node_replace(startnode, root)) != root)
     free (root);
 
   free ($1);
@@ -134,14 +138,12 @@ sentence_root: WORD
 cmd_token:
   placeholder_token
 {
-  currnode = add_node(currnode, $1);
-  if (currnode != $1)
+  if ((currnode = node_replace(currnode, $1)) != $1)
     free_node ($1);
 }
 | literal_token
 {
-  currnode = add_node(currnode, $1);
-  if (currnode != $1)
+  if ((currnode = node_replace(currnode, $1)) != $1)
     free_node ($1);
 }
 /* selectors and options are subgraphs with start and end nodes */
@@ -226,7 +228,7 @@ literal_token:
 
 /* <selector|set> productions */
 selector:
-  '<' selector_part '|' selector_element '>'
+  '<' selector_part '>'
 {
   // all the graph building is done in selector_element,
   // so just return the selector subgraph head
@@ -235,7 +237,7 @@ selector:
 
 selector_part:
   selector_part '|' selector_element
-| selector_element
+| selector_element '|' selector_element
 ;
 
 selector_element:
@@ -292,10 +294,7 @@ selector_token:
 option: '[' option_part ']'
 {
   // add null path
-  struct graph_node *nullpath = new_node(NUL_GN);
-  add_node(optnode_start, nullpath);
-  add_node(nullpath, optnode_end);
-
+  add_node(optnode_start, optnode_end);
   $$ = optnode_start;
 };
 
@@ -353,7 +352,7 @@ parse_command_format(struct graph_node *start, struct cmd_element *cmd)
   optnode_start = optnode_end = NULL;
 
   // trace parser
-  yydebug = 0;
+  yydebug = 1;
   // command string
   command = cmd;
   // make flex read from a string
@@ -363,3 +362,26 @@ parse_command_format(struct graph_node *start, struct cmd_element *cmd)
   // startnode points to command DFA
   return startnode;
 }
+
+struct graph_node *
+node_exists(struct graph_node *parent, struct graph_node *child)
+{
+  struct graph_node *p_child;
+  for (unsigned int i = 0; i < vector_active(parent->children); i++)
+  {
+    p_child = vector_slot(parent->children, i);
+    if (cmp_node(child, p_child))
+      return p_child;
+  }
+  return NULL;
+}
+
+struct graph_node *
+node_replace(struct graph_node *parent, struct graph_node *child)
+{
+  struct graph_node *existing = node_exists (parent, child);
+  if (!existing)
+    existing = add_node(parent, child);
+
+  return existing;
+}