Filter refactoring: Passing the resulting struct f_val as a pointer.

This also drops the multiplexing of errors with the f_val itself
together with the T_RETURN f_val type flag.

7 files changed, 60 insertions, 61 deletions

diff --git a/conf/confbase.Y b/conf/confbase.Y
index 3fd034db..3d573e10 100644
--- a/conf/confbase.Y
+++ b/conf/confbase.Y
@@ -124,8 +124,7 @@ conf: definition ;
 definition:
    DEFINE SYM '=' term ';' {
      struct f_val *val = cfg_alloc(sizeof(struct f_val));
-     *val = f_eval($4, cfg_mem);
-     if (val->type == T_RETURN) cf_error("Runtime error");
+     if (f_eval($4, cfg_mem, val) > F_RETURN) cf_error("Runtime error");
      cf_define_symbol($2, SYM_CONSTANT | val->type, val);
    }
  ;
diff --git a/filter/config.Y b/filter/config.Y
index c1e74531..200c1f41 100644
--- a/filter/config.Y
+++ b/filter/config.Y
@@ -653,7 +653,7 @@ set_atom:
  | VPN_RD { $$.type = T_RD; $$.val.ec = $1; }
  | ENUM   { $$.type = pair_a($1); $$.val.i = pair_b($1); }
  | '(' term ')' {
-     $$ = f_eval($2, cfg_mem);
+     if (f_eval($2, cfg_mem, &($$)) > F_RETURN) cf_error("Runtime error");
      if (!f_valid_set_type($$.type)) cf_error("Set-incompatible type");
    }
  | SYM {
diff --git a/filter/f-inst.c b/filter/f-inst.c
index 24908442..814e026d 100644
--- a/filter/f-inst.c
+++ b/filter/f-inst.c
@@ -121,15 +121,15 @@
       while (tt) {
 	*vv = lp_alloc(fs->pool, sizeof(struct f_path_mask));
 	if (tt->kind == PM_ASN_EXPR) {
-	  struct f_val res;
-	  INTERPRET(res, (struct f_inst *) tt->val);
+	  struct f_val xres;
+	  INTERPRET(xres, (struct f_inst *) tt->val);
 	  (*vv)->kind = PM_ASN;
-	  if (res.type != T_INT) {
+	  if (xres.type != T_INT) {
 	    runtime( "Error resolving path mask template: value not an integer" );
-	    return (struct f_val) { .type = T_VOID };
+	    return F_ERROR;
 	  }
 
-	  (*vv)->val = res.val.i;
+	  (*vv)->val = xres.val.i;
 	} else {
 	  **vv = *tt;
 	}
@@ -280,9 +280,7 @@
       /* Should take care about turning ACCEPT into MODIFY */
     case F_ERROR:
     case F_REJECT:	/* FIXME (noncritical) Should print complete route along with reason to reject route */
-      res.type = T_RETURN;
-      res.val.i = what->a2.i;
-      return res;	/* We have to return now, no more processing. */
+      return what->a2.i;	/* We have to return now, no more processing. */
     case F_NONL:
     case F_NOP:
       break;
@@ -658,14 +656,12 @@
   case FI_RETURN:
     ARG_ANY(1);
     res = v1;
-    res.type |= T_RETURN;
-    return res;
-  case FI_CALL: /* CALL: this is special: if T_RETURN and returning some value, mask it out  */
+    return F_RETURN;
+  case FI_CALL:
     ARG_ANY(1);
-    res = interpret(fs, what->a2.p);
-    if (res.type == T_RETURN)
-      return res;
-    res.type &= ~T_RETURN;
+    fret = interpret(fs, what->a2.p, &res);
+    if (fret > F_RETURN)
+      return fret;
     break;
   case FI_CLEAR_LOCAL_VARS:	/* Clear local variables */
     for (sym = what->a1.p; sym != NULL; sym = sym->aux2)
diff --git a/filter/filter.c b/filter/filter.c
index 794e35e8..1e6fc003 100644
--- a/filter/filter.c
+++ b/filter/filter.c
@@ -615,24 +615,27 @@ static struct tbf rl_runtime_err = TBF_DEFAULT_LOG_LIMITS;
  * &f_val structures are copied around, so there are no problems with
  * memory managment.
  */
-static struct f_val
-interpret(struct filter_state *fs, struct f_inst *what)
+static enum filter_return
+interpret(struct filter_state *fs, struct f_inst *what, struct f_val *resp)
 {
   struct symbol *sym;
-  struct f_val v1, v2, v3, res = { .type = T_VOID }, *vp;
+  struct f_val v1, v2, v3, *vp;
   unsigned u1, u2;
+  enum filter_return fret;
   int i;
   u32 as;
 
+  *resp = (struct f_val) { .type = T_VOID };
+
   for ( ; what; what = what->next) {
-  res.type = T_VOID;
+#define res (*resp)
+  res = (struct f_val) { .type = T_VOID };
   switch(what->fi_code) {
+
 #define runtime(fmt, ...) do { \
     if (!(fs->flags & FF_SILENT)) \
       log_rl(&rl_runtime_err, L_ERR "filters, line %d: " fmt, what->lineno, ##__VA_ARGS__); \
-    res.type = T_RETURN; \
-    res.val.i = F_ERROR; \
-    return res; \
+    return F_ERROR; \
   } while(0)
 
 #define ARG_ANY(n) INTERPRET(v##n, what->a##n.p)
@@ -643,9 +646,9 @@ interpret(struct filter_state *fs, struct f_inst *what)
 	  n, f_instruction_name(what->fi_code), t, v##n.type);
 
 #define INTERPRET(val, what_) \
-    val = interpret(fs, what_); \
-    if (val.type & T_RETURN) \
-      return val;
+    fret = interpret(fs, what_, &(val)); \
+    if (fret >= F_RETURN) \
+      return fret;
 
 #define ACCESS_RTE \
   do { if (!fs->rte) runtime("No route to access"); } while (0)
@@ -657,6 +660,7 @@ interpret(struct filter_state *fs, struct f_inst *what)
 
 #include "filter/f-inst.c"
 
+#undef res
 #undef runtime
 #undef ARG_ANY
 #undef ARG
@@ -664,7 +668,7 @@ interpret(struct filter_state *fs, struct f_inst *what)
 #undef ACCESS_RTE
 #undef ACCESS_EATTRS
   }}
-  return res;
+  return F_NOP;
 }
 
 
@@ -837,7 +841,7 @@ i_same(struct f_inst *f1, struct f_inst *f2)
  * (and cached rta of read-only source rte is intact), if rte is
  * modified in place, old cached rta is possibly freed.
  */
-int
+enum filter_return
 f_run(struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags)
 {
   if (filter == FILTER_ACCEPT)
@@ -857,7 +861,8 @@ f_run(struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int fla
 
   LOG_BUFFER_INIT(fs.buf);
 
-  struct f_val res = interpret(&fs, filter->root);
+  struct f_val res;
+  enum filter_return fret = interpret(&fs, filter->root, &res);
 
   if (fs.old_rta) {
     /*
@@ -879,18 +884,18 @@ f_run(struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int fla
   }
 
 
-  if (res.type != T_RETURN) {
+  if (fret < F_ACCEPT) {
     if (!(fs.flags & FF_SILENT))
       log_rl(&rl_runtime_err, L_ERR "Filter %s did not return accept nor reject. Make up your mind", filter->name);
     return F_ERROR;
   }
   DBG( "done (%u)\n", res.val.i );
-  return res.val.i;
+  return fret;
 }
 
 /* TODO: perhaps we could integrate f_eval(), f_eval_rte() and f_run() */
 
-struct f_val
+enum filter_return
 f_eval_rte(struct f_inst *expr, struct rte **rte, struct linpool *tmp_pool)
 {
 
@@ -902,13 +907,12 @@ f_eval_rte(struct f_inst *expr, struct rte **rte, struct linpool *tmp_pool)
   LOG_BUFFER_INIT(fs.buf);
 
   /* Note that in this function we assume that rte->attrs is private / uncached */
-  struct f_val res = interpret(&fs, expr);
-
-  return res;
+  struct f_val res;
+  return interpret(&fs, expr, &res);
 }
 
-struct f_val
-f_eval(struct f_inst *expr, struct linpool *tmp_pool)
+enum filter_return
+f_eval(struct f_inst *expr, struct linpool *tmp_pool, struct f_val *pres)
 {
   struct filter_state fs = {
     .pool = tmp_pool,
@@ -916,14 +920,16 @@ f_eval(struct f_inst *expr, struct linpool *tmp_pool)
 
   LOG_BUFFER_INIT(fs.buf);
 
-  return interpret(&fs, expr);
+  return interpret(&fs, expr, pres);
 }
 
 uint
 f_eval_int(struct f_inst *expr)
 {
   /* Called independently in parse-time to eval expressions */
-  struct f_val res = f_eval(expr, cfg_mem);
+  struct f_val res;
+  if (f_eval(expr, cfg_mem, &res) > F_RETURN)
+    cf_error("Runtime error while evaluating expression");
 
   if (res.type != T_INT)
     cf_error("Integer expression expected");
diff --git a/filter/filter.h b/filter/filter.h
index a8c33287..8728f679 100644
--- a/filter/filter.h
+++ b/filter/filter.h
@@ -175,9 +175,19 @@ void trie_format(struct f_trie *t, buffer *buf);
 struct ea_list;
 struct rte;
 
-int f_run(struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags);
-struct f_val f_eval_rte(struct f_inst *expr, struct rte **rte, struct linpool *tmp_pool);
-struct f_val f_eval(struct f_inst *expr, struct linpool *tmp_pool);
+enum filter_return {
+  F_NOP = 0,
+  F_NONL,
+  F_RETURN,
+  F_ACCEPT,   /* Need to preserve ordering: accepts < rejects! */
+  F_REJECT,
+  F_ERROR,
+  F_QUITBIRD,
+};
+
+enum filter_return f_run(struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags);
+enum filter_return f_eval_rte(struct f_inst *expr, struct rte **rte, struct linpool *tmp_pool);
+enum filter_return f_eval(struct f_inst *expr, struct linpool *tmp_pool, struct f_val *pres);
 uint f_eval_int(struct f_inst *expr);
 
 char *filter_name(struct filter *filter);
@@ -190,14 +200,6 @@ int val_same(struct f_val v1, struct f_val v2);
 
 void val_format(struct f_val v, buffer *buf);
 
-
-#define F_NOP 0
-#define F_NONL 1
-#define F_ACCEPT 2	/* Need to preserve ordering: accepts < rejects! */
-#define F_REJECT 3
-#define F_ERROR 4
-#define F_QUITBIRD 5
-
 #define FILTER_ACCEPT NULL
 #define FILTER_REJECT ((void *) 1)
 #define FILTER_UNDEF  ((void *) 2)	/* Used in BGP */
@@ -246,7 +248,6 @@ void val_format(struct f_val v, buffer *buf);
 #define T_LCLIST 0x29		/* Large community list */
 #define T_RD 0x2a		/* Route distinguisher for VPN addresses */
 
-#define T_RETURN 0x40
 #define T_SET 0x80
 #define T_PREFIX_SET 0x81
 
diff --git a/filter/filter_test.c b/filter/filter_test.c
index be7fd521..b319225b 100644
--- a/filter/filter_test.c
+++ b/filter/filter_test.c
@@ -44,13 +44,11 @@ run_function(const void *parsed_fn_def)
   struct f_inst *f = (struct f_inst *) parsed_fn_def;
 
   linpool *tmp = lp_new_default(&root_pool);
-  struct f_val res = f_eval(f, tmp);
+  struct f_val res;
+  enum filter_return fret = f_eval(f, tmp, &res);
   rfree(tmp);
 
-  if (res.type == T_RETURN && res.val.i >= F_REJECT)
-    return 0;
-
-  return 1;
+  return (fret < F_REJECT);
 }
 
 static void
diff --git a/nest/cmds.c b/nest/cmds.c
index ca601ef2..0c89d20f 100644
--- a/nest/cmds.c
+++ b/nest/cmds.c
@@ -97,9 +97,8 @@ cmd_show_memory(void)
 void
 cmd_eval(struct f_inst *expr)
 {
-  struct f_val v = f_eval(expr, this_cli->parser_pool);
-
-  if (v.type == T_RETURN)
+  struct f_val v;
+  if (f_eval(expr, this_cli->parser_pool, &v) > F_RETURN)
     {
       cli_msg(8008, "runtime error");
       return;