vlib: debug CLI macro expander, part deux

Deal with circular macro definitions instead of crashing due to stack
overflow.

Separate macro tables, per CLI session

Add documentation to the Sphinx docs

Type: improvement

Signed-off-by: Dave Barach <dave@barachs.net>
Change-Id: I55fc9152bd37ad0c15fa3959f38b07b63100e634

diff --git a/docs/gettingstarted/developers/vlib.md b/docs/gettingstarted/developers/vlib.md
index 13844ab..59206e5 100644 (file)
--- a/docs/gettingstarted/developers/vlib.md
+++ b/docs/gettingstarted/developers/vlib.md
@@ -540,6 +540,54 @@ certain cli command has the potential to hurt packet processing
 performance by running for too long, do the work incrementally in a
 process node. The client can wait.
 
+### Macro expansion
+
+The vpp debug CLI engine includes a recursive macro expander. This
+is quite useful for factoring out address and/or interface name
+specifics:
+
+```
+   define ip1 192.168.1.1/24
+   define ip2 192.168.2.1/24
+   define iface1 GigabitEthernet3/0/0
+   define iface2 loop1
+
+   set int ip address $iface1 $ip1
+   set int ip address $iface2 $(ip2)
+
+   undefine ip1
+   undefine ip2
+   undefine iface1
+   undefine iface2
+```
+
+Each socket (or telnet) debug CLI session has its own macro
+tables. All debug CLI sessions which use CLI_INBAND binary API
+messages share a single table.
+
+The macro expander recognizes circular defintions:
+
+```
+    define foo \$(bar)
+    define bar \$(mumble)
+    define mumble \$(foo)
+```
+
+At 8 levels of recursion, the macro expander throws up its hands and
+replies "CIRCULAR."
+
+### Macro-related debug CLI commands
+
+In addition to the "define" and "undefine" debug CLI commands, use
+"show macro [noevaluate]" to dump the macro table. The "echo" debug
+CLI command will evaluate and print its argument:
+
+```
+    vpp# define foo This\ Is\ Foo
+    vpp# echo $foo
+    This Is Foo
+```
+
 Handing off buffers between threads
 -----------------------------------
 

diff --git a/src/vat/main.c b/src/vat/main.c
index a6779c5..8a7ed63 100644 (file)
--- a/src/vat/main.c
+++ b/src/vat/main.c
@@ -108,7 +108,9 @@ do_one_file (vat_main_t * vam)
 
       this_cmd =
        (u8 *) clib_macro_eval (&vam->macro_main, (i8 *) vam->inbuf,
-                               1 /* complain */ );
+                               1 /* complain */ ,
+                               0 /* level */ ,
+                               8 /* max_level */ );
 
       if (vam->exec_mode == 0)
        {

diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c
index 6b02fdf..5979dbd 100644 (file)
--- a/src/vlib/unix/cli.c
+++ b/src/vlib/unix/cli.c
@@ -242,6 +242,8 @@ typedef struct
    */
   u8 cursor_direction;
 
+  /** Macro tables for this session */
+  clib_macro_main_t macro_main;
 } unix_cli_file_t;
 
 /** Resets the pager buffer and other data.
@@ -493,7 +495,7 @@ typedef struct
   /** List of new sessions */
   unix_cli_new_session_t *new_sessions;
 
-  /* Macro expander */
+  /** system default macro table */
   clib_macro_main_t macro_main;
 
 } unix_cli_main_t;
@@ -501,6 +503,24 @@ typedef struct
 /** CLI global state */
 static unix_cli_main_t unix_cli_main;
 
+/** Return the macro main / tables we should use for this session
+ */
+static clib_macro_main_t *
+get_macro_main (void)
+{
+  unix_cli_main_t *cm = &unix_cli_main;
+  vlib_main_t *vm = vlib_get_main ();
+  vlib_process_t *cp = vlib_get_current_process (vm);
+  unix_cli_file_t *cf;
+
+  if (pool_is_free_index (cm->cli_file_pool, cp->output_function_arg))
+    return (&cm->macro_main);
+
+  cf = pool_elt_at_index (cm->cli_file_pool, cp->output_function_arg);
+
+  return (&cf->macro_main);
+}
+
 /**
  * @brief Search for a byte sequence in the action list.
  *
@@ -2575,9 +2595,11 @@ more:
       vec_validate (cf->current_command, vec_len (cf->current_command));
       cf->current_command[vec_len (cf->current_command) - 1] = 0;
       /* The macro expander expects proper C-strings, not vectors */
-      expanded = (u8 *) clib_macro_eval (&cm->macro_main,
+      expanded = (u8 *) clib_macro_eval (&cf->macro_main,
                                         (i8 *) cf->current_command,
-                                        1 /* complain */ );
+                                        1 /* complain */ ,
+                                        0 /* level */ ,
+                                        8 /* max_level */ );
       /* Macro processor NULL terminates the return */
       _vec_len (expanded) -= 1;
       vec_reset_length (cf->current_command);
@@ -2689,6 +2711,7 @@ unix_cli_kill (unix_cli_main_t * cm, uword cli_file_index)
   clib_file_del (fm, uf);
 
   unix_cli_file_free (cf);
+  clib_macro_free (&cf->macro_main);
   pool_put (cm->cli_file_pool, cf);
 }
 
@@ -2906,6 +2929,7 @@ unix_cli_file_add (unix_cli_main_t * cm, char *name, int fd)
 
   pool_get (cm->cli_file_pool, cf);
   clib_memset (cf, 0, sizeof (*cf));
+  clib_macro_init (&cf->macro_main);
 
   template.read_function = unix_cli_read_ready;
   template.write_function = unix_cli_write_ready;
@@ -3962,7 +3986,7 @@ define_cmd_fn (vlib_main_t * vm,
 {
   u8 *macro_name;
   unformat_input_t _line_input, *line_input = &_line_input;
-  unix_cli_main_t *cm = &unix_cli_main;
+  clib_macro_main_t *mm = get_macro_main ();
   clib_error_t *error;
 
   if (!unformat (input, "%s", &macro_name))
@@ -3980,8 +4004,7 @@ define_cmd_fn (vlib_main_t * vm,
     }
   /* the macro expander expects c-strings, not vectors... */
   vec_add1 (line_input->buffer, 0);
-  clib_macro_set_value (&cm->macro_main, (char *) macro_name,
-                       (char *) line_input->buffer);
+  clib_macro_set_value (mm, (char *) macro_name, (char *) line_input->buffer);
   vec_free (macro_name);
   unformat_free (line_input);
   return 0;
@@ -4001,12 +4024,12 @@ undefine_cmd_fn (vlib_main_t * vm,
                 unformat_input_t * input, vlib_cli_command_t * cmd)
 {
   u8 *macro_name;
-  unix_cli_main_t *cm = &unix_cli_main;
+  clib_macro_main_t *mm = get_macro_main ();
 
   if (!unformat (input, "%s", &macro_name))
     return clib_error_return (0, "missing variable name...");
 
-  if (clib_macro_unset (&cm->macro_main, (char *) macro_name))
+  if (clib_macro_unset (mm, (char *) macro_name))
     vlib_cli_output (vm, "%s wasn't set...", macro_name);
 
   vec_free (macro_name);
@@ -4025,7 +4048,7 @@ static clib_error_t *
 show_macro_cmd_fn (vlib_main_t * vm,
                   unformat_input_t * input, vlib_cli_command_t * cmd)
 {
-  unix_cli_main_t *cm = &unix_cli_main;
+  clib_macro_main_t *mm = get_macro_main ();
   int evaluate = 1;
 
   if (unformat (input, "noevaluate %=", &evaluate, 0))
@@ -4033,8 +4056,7 @@ show_macro_cmd_fn (vlib_main_t * vm,
   else if (unformat (input, "noeval %=", &evaluate, 0))
     ;
 
-  vlib_cli_output (vm, "%U", format_clib_macro_main, &cm->macro_main,
-                  evaluate);
+  vlib_cli_output (vm, "%U", format_clib_macro_main, mm, evaluate);
   return 0;
 }
 
@@ -4055,6 +4077,7 @@ unix_cli_init (vlib_main_t * vm)
    * has not been started */
   cm->new_session_process_node_index = ~0;
   clib_macro_init (&cm->macro_main);
+
   return 0;
 }
 

diff --git a/src/vppinfra/macros.c b/src/vppinfra/macros.c
index bc6df55..240cef0 100644 (file)
--- a/src/vppinfra/macros.c
+++ b/src/vppinfra/macros.c
@@ -92,12 +92,20 @@ clib_macro_get_value (clib_macro_main_t * mm, char *name)
  * looks up $foobar in the variable table.
  */
 i8 *
-clib_macro_eval (clib_macro_main_t * mm, i8 * s, i32 complain)
+clib_macro_eval (clib_macro_main_t * mm, i8 * s, i32 complain, u16 level,
+                u16 max_level)
 {
   i8 *rv = 0;
   i8 *varname, *varvalue;
   i8 *ts;
 
+  if (level >= max_level)
+    {
+      if (complain)
+       clib_warning ("circular definition, level %d", level);
+      return (i8 *) format (0, " CIRCULAR ");
+    }
+
   while (*s)
     {
       switch (*s)
@@ -161,7 +169,8 @@ clib_macro_eval (clib_macro_main_t * mm, i8 * s, i32 complain)
          if (varvalue)
            {
              /* recursively evaluate */
-             ts = clib_macro_eval (mm, varvalue, complain);
+             ts = clib_macro_eval (mm, varvalue, complain, level + 1,
+                                   max_level);
              vec_free (varvalue);
              /* add results to answer */
              vec_append (rv, ts);
@@ -195,7 +204,7 @@ clib_macro_eval_dollar (clib_macro_main_t * mm, i8 * s, i32 complain)
   i8 *rv;
 
   s2 = (i8 *) format (0, "$(%s)%c", s, 0);
-  rv = clib_macro_eval (mm, s2, complain);
+  rv = clib_macro_eval (mm, s2, complain, 0 /* level */ , 8 /* max_level */ );
   vec_free (s2);
   return (rv);
 }

diff --git a/src/vppinfra/macros.h b/src/vppinfra/macros.h
index 54ceda7..1b2064a 100644 (file)
--- a/src/vppinfra/macros.h
+++ b/src/vppinfra/macros.h
@@ -39,7 +39,8 @@ int clib_macro_set_value (clib_macro_main_t * mm, char *name, char *value);
 void clib_macro_add_builtin (clib_macro_main_t * mm, char *name,
                             void *eval_fn);
 i8 *clib_macro_get_value (clib_macro_main_t * mm, char *name);
-i8 *clib_macro_eval (clib_macro_main_t * mm, i8 * s, i32 complain);
+i8 *clib_macro_eval (clib_macro_main_t * mm, i8 * s, i32 complain,
+                    u16 level, u16 max_level);
 i8 *clib_macro_eval_dollar (clib_macro_main_t * mm, i8 * s, i32 complain);
 void clib_macro_init (clib_macro_main_t * mm);
 void clib_macro_free (clib_macro_main_t * mm);