lib: Cleanup parser memory management

Free as appropriate. Additionally add new type
of node to demark graph head (START_GN).

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

lib/command_graph.c

@@ -32,18 +32,18 @@ cmp_node(struct graph_node *first, struct graph_node *second)
   if (first->type != second->type) return 0;
 
   switch (first->type) {
-    case WORD_GN:       // words and variables are equal if their
+    case WORD_GN:
-    case VARIABLE_GN:   // text value is equal
+    case VARIABLE_GN:
       if (first->text && second->text) {
         if (strcmp(first->text, second->text)) return 0;
       }
       else if (first->text != second->text) return 0;
       break;
-    case RANGE_GN:      // ranges are equal if their bounds are equal
+    case RANGE_GN:
       if (first->min != second->min || first->max != second->max)
         return 0;
       break;
-    case NUMBER_GN:     // numbers are equal if their values are equal
+    case NUMBER_GN:
       if (first->value != second->value) return 0;
       break;
     /* selectors and options should be equal if all paths are equal,
@@ -53,8 +53,10 @@ cmp_node(struct graph_node *first, struct graph_node *second)
     case SELECTOR_GN:
     case OPTION_GN:
       return 0;
-    // end nodes are always considered equal, since each node may only
+    /* end nodes are always considered equal, since each node may only
-    // have one at a time
+     * have one at a time
+     */
+    case START_GN:
     case END_GN:
     default:
       break;
@@ -66,20 +68,34 @@ cmp_node(struct graph_node *first, struct graph_node *second)
 struct graph_node *
 new_node(enum graph_node_type type)
 {
-  struct graph_node *node = malloc(sizeof(struct graph_node));
+  struct graph_node *node =
+     XMALLOC(MTYPE_CMD_TOKENS, sizeof(struct graph_node));
+
   node->type = type;
   node->children = vector_init(VECTOR_MIN_SIZE);
-  node->is_root = 0;
+  node->is_start = 0;
-  node->end = NULL;
+  node->end      = NULL;
-  node->text = NULL;
+  node->text     = NULL;
-  node->value = 0;
+  node->value    = 0;
-  node->min   = 0;
+  node->min      = 0;
-  node->max   = 0;
+  node->max      = 0;
-  node->element = NULL;
+  node->element  = NULL;
 
   return node;
 }
 
+void
+free_node (struct graph_node *node)
+{
+  if (!node) return;
+  free_node (node->end);
+  vector_free (node->children);
+  free (node->text);
+  free (node->arg);
+  free (node->element);
+  free (node);
+}
+
 char *
 describe_node(struct graph_node *node, char* buffer, unsigned int bufsize)
 {
@@ -114,6 +130,9 @@ describe_node(struct graph_node *node, char* buffer, unsigned int bufsize)
     case END_GN:
       snprintf(buffer, bufsize, "END");
       break;
+    case START_GN:
+      snprintf(buffer, bufsize, "START");
+      break;
     default:
       snprintf(buffer, bufsize, "ERROR");
   }
@@ -146,3 +165,4 @@ walk_graph(struct graph_node *start, int level)
   else
     fprintf(stderr, "\n");
 }
+

lib/command_graph.h

@@ -16,6 +16,7 @@ enum graph_node_type
   SELECTOR_GN,
   OPTION_GN,
   NUL_GN,
+  START_GN,
   END_GN
 };
 
@@ -26,9 +27,9 @@ struct graph_node
   vector children;          // this node's children
   struct graph_node * end;  // pointer to end for SELECTOR_GN & OPTION_GN
 
-  char* text;       // for WORD_GN and VARIABLE_GN
+  char* text;               // for WORD_GN and VARIABLE_GN
-  long value;       // for NUMBER_GN
+  long value;               // for NUMBER_GN
-  long min, max;    // for RANGE_GN
+  long min, max;            // for RANGE_GN
 
   /* cmd_element struct pointer, only valid for END_GN */
   struct cmd_element *element;
@@ -37,14 +38,14 @@ struct graph_node
 };
 
 /*
- * Adds a child to a node.
+ * Adds a node as a child of another node.
- * If the node already has the exact same child, nothing is done. This is
+ * If the new parent has a child that is equal to the prospective child, as
- * decided with cmp_node.
+ * determined by cmp_node, then a pointer to the existing node is returned and
+ * the prospective child is not added. Otherwise the return value is NULL.
  *
  * @param[in] parent node
  * @param[in] child node
- * @return the new child, or the existing child if the parent already has the
+ * @return pointer to child if it is added, pointer to existing child otherwise
- *         new child
  */
 extern struct graph_node *
 add_node(struct graph_node *, struct graph_node *);

lib/command_parse.y

@@ -18,6 +18,7 @@ extern void yyerror(const char *);
 %code requires {
   #include "command.h"
   #include "command_graph.h"
+  #include "memory.h"
 }
 %code provides {
   extern void
@@ -50,14 +51,14 @@ struct graph_node *selnode_start,   // start node for selector set
 struct cmd_element *command;        // command we're parsing
 %}
 
-%token <node> WORD
+%token <string> WORD
-%token <node> IPV4
+%token <string> IPV4
-%token <node> IPV4_PREFIX
+%token <string> IPV4_PREFIX
-%token <node> IPV6
+%token <string> IPV6
-%token <node> IPV6_PREFIX
+%token <string> IPV6_PREFIX
-%token <node> VARIABLE
+%token <string> VARIABLE
-%token <node> RANGE
+%token <string> RANGE
-%token <node> NUMBER
+%token <integer> NUMBER
 
 %type <node> start
 %type <node> sentence_root
@@ -80,44 +81,45 @@ struct cmd_element *command;        // command we're parsing
 start: sentence_root
        cmd_token_seq
 {
-  // this should never happen...
+  // create leaf node
-  if (currnode->type == END_GN)
-    yyerror("Unexpected leaf");
-
-  // create function pointer node
   struct graph_node *end = new_node(END_GN);
   end->element = command;
 
-  // ensure there are no END_GN children
-  for (unsigned int i = 0; i < vector_active(currnode->children); i++)
-  {
-    struct graph_node *child = vector_slot(currnode->children, i);
-    if (child->type == END_GN)
-      yyerror("Duplicate command.");
-  }
-
   // add node
-  end = add_node(currnode, end);
+  if (add_node(currnode, end) != end)
+  {
+    yyerror("Duplicate command.");
+    YYABORT;
+  }
 }
 
 sentence_root: WORD
 {
-  $$ = new_node(WORD_GN);
+  struct graph_node *root = new_node(WORD_GN);
-  $$->text = strdup(yylval.string);
+  root->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
-  fprintf(stderr, ">>>>>>>> YYLVAL.STRING: %s\n", yylval.string);
-  fprintf(stderr, ">>>>>>>>          TEXT: %s\n", $$->text);
 
-  currnode = $$;
+  currnode = add_node(startnode, root);
-  currnode->is_root = 1;
+  if (currnode != root)
-  currnode = add_node(startnode, currnode);
+    free (root);
+
+  free ($1);
+  $$ = currnode;
 };
 
 /* valid top level tokens */
 cmd_token:
   placeholder_token
-{ currnode = add_node(currnode, $1); }
+{
+  currnode = add_node(currnode, $1);
+  if (currnode != $1)
+    free_node ($1);
+}
 | literal_token
-{ currnode = add_node(currnode, $1); }
+{
+  currnode = add_node(currnode, $1);
+  if (currnode != $1)
+    free_node ($1);
+}
 /* selectors and options are subgraphs with start and end nodes */
 | selector
 {
@@ -142,37 +144,44 @@ placeholder_token:
   IPV4
 {
   $$ = new_node(IPV4_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
+  free ($1);
 }
 | IPV4_PREFIX
 {
   $$ = new_node(IPV4_PREFIX_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
+  free ($1);
 }
 | IPV6
 {
   $$ = new_node(IPV6_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
+  free ($1);
 }
 | IPV6_PREFIX
 {
   $$ = new_node(IPV6_PREFIX_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
+  free ($1);
 }
 | VARIABLE
 {
   $$ = new_node(VARIABLE_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
+  free ($1);
 }
 | RANGE
 {
   $$ = new_node(RANGE_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
 
   // get the numbers out
   strsep(&yylval.string, "(-)");
   $$->min = atoi( strsep(&yylval.string, "(-)") );
   $$->max = atoi( strsep(&yylval.string, "(-)") );
+
+  free ($1);
 }
 ;
 
@@ -180,9 +189,8 @@ literal_token:
   WORD
 {
   $$ = new_node(WORD_GN);
-  $$->text = strdup(yylval.string);
+  $$->text = XSTRDUP(MTYPE_CMD_TOKENS, $1);
-  fprintf(stderr, ">>>>>>>> YYLVAL.STRING: %s\n", yylval.string);
+  free ($1);
-  fprintf(stderr, ">>>>>>>>          TEXT: %s\n", $$->text);
 }
 | NUMBER
 {
@@ -300,8 +308,7 @@ option_token:
 
 void yyerror(char const *message) {
   // fail on bad parse
-  printf("Grammar error: %s\n", message);
+  fprintf(stderr, "Grammar error: %s\n", message);
-  exit(EXIT_FAILURE);
 }
 
 struct graph_node *
@@ -310,7 +317,8 @@ parse_command_format(struct graph_node *start, struct cmd_element *cmd)
   fprintf(stderr, "parsing: %s\n", cmd->string);
 
   /* clear state pointers */
-  startnode = currnode = seqhead = NULL;
+  startnode = start;
+  currnode = seqhead = NULL;
   selnode_start = selnode_end = NULL;
   optnode_start = optnode_end = NULL;
 
@@ -320,8 +328,6 @@ parse_command_format(struct graph_node *start, struct cmd_element *cmd)
   command = cmd;
   // make flex read from a string
   set_buffer_string(command->string);
-  // initialize the start node of this command dfa
-  startnode = start;
   // parse command into DFA
   yyparse();
   // startnode points to command DFA

lib/grammar_sandbox.c

@@ -101,7 +101,7 @@ DEFUN (grammar_test_match,
 void grammar_sandbox_init(void);
 void grammar_sandbox_init() {
   fprintf(stderr, "reinitializing graph\n");
-  nodegraph = new_node(NUL_GN);
+  nodegraph = new_node(START_GN);
   install_element (ENABLE_NODE, &grammar_test_cmd);
   install_element (ENABLE_NODE, &grammar_test_show_cmd);
   install_element (ENABLE_NODE, &grammar_test_match_cmd);