src: Simplify parser rule_spec tree

This patch separates the rule identification from the rule localization,
so the logic moves from the evaluator to the parser. This allows to
revert the patch "evaluate: improve rule managment checks"
(4176c7d30c2ff1b3f52468fc9c08b8df83f979a8) and saves a lot of code.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/evaluate.c b/src/evaluate.c
index 87f5a6d77d485eefdcc034490644daabd3b90736..2f94ac6e39a8dd26c8ac58ead3b50a34dc830dba 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -44,12 +44,6 @@ static const char *byteorder_names[] = {
        __stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
 #define cmd_error(ctx, fmt, args...) \
        __stmt_binary_error(ctx, &(ctx->cmd)->location, NULL, fmt, ## args)
-#define handle_error(ctx, fmt, args...) \
-       __stmt_binary_error(ctx, &ctx->cmd->handle.handle.location, NULL, fmt, ## args)
-#define position_error(ctx, fmt, args...) \
-       __stmt_binary_error(ctx, &ctx->cmd->handle.position.location, NULL, fmt, ## args)
-#define handle_position_error(ctx, fmt, args...) \
-       __stmt_binary_error(ctx, &ctx->cmd->handle.handle.location, &ctx->cmd->handle.position.location, fmt, ## args)
 
 static int __fmtstring(3, 4) set_error(struct eval_ctx *ctx,
                                       const struct set *set,
@@ -2481,68 +2475,11 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
        return 0;
 }
 
-static int rule_evaluate_cmd(struct eval_ctx *ctx)
-{
-       struct handle *handle = &ctx->cmd->handle;
-
-       /* allowed:
-        * - insert [position] (no handle)
-        * - add [position] (no handle)
-        * - replace <handle> (no position)
-        * - delete <handle> (no position)
-        */
-
-       switch (ctx->cmd->op) {
-       case CMD_INSERT:
-               if (handle->handle.id && handle->position.id)
-                       return handle_position_error(ctx, "use only `position'"
-                                                    " instead");
-
-               if (handle->handle.id)
-                       return handle_error(ctx, "use `position' instead");
-               break;
-       case CMD_ADD:
-               if (handle->handle.id && handle->position.id)
-                       return handle_position_error(ctx, "use only `position'"
-                                                    " instead");
-
-               if (handle->handle.id)
-                       return handle_error(ctx, "use `position' instead");
-
-               break;
-       case CMD_REPLACE:
-               if (handle->handle.id && handle->position.id)
-                       return handle_position_error(ctx, "use only `handle' "
-                                                    "instead");
-               if (handle->position.id)
-                       return position_error(ctx, "use `handle' instead");
-               if (!handle->handle.id)
-                       return cmd_error(ctx, "missing `handle'");
-               break;
-       case CMD_DELETE:
-               if (handle->handle.id && handle->position.id)
-                       return handle_position_error(ctx, "use only `handle' "
-                                                    "instead");
-               if (handle->position.id)
-                       return position_error(ctx, "use `handle' instead");
-               if (!handle->handle.id)
-                       return cmd_error(ctx, "missing `handle'");
-               break;
-       default:
-               BUG("unkown command type %u\n", ctx->cmd->op);
-       }
-
-       return 0;
-}
-
 static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule)
 {
        struct stmt *stmt, *tstmt = NULL;
        struct error_record *erec;
 
-       if (rule_evaluate_cmd(ctx) < 0)
-               return -1;
-
        proto_ctx_init(&ctx->pctx, rule->handle.family);
        memset(&ctx->ectx, 0, sizeof(ctx->ectx));
 
@@ -2723,11 +2660,8 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
                        return ret;
 
                return setelem_evaluate(ctx, &cmd->expr);
-       case CMD_OBJ_RULE:
-               if (rule_evaluate_cmd(ctx) < 0)
-                       return -1;
-               /* fall through */
        case CMD_OBJ_SET:
+       case CMD_OBJ_RULE:
        case CMD_OBJ_CHAIN:
        case CMD_OBJ_TABLE:
                return 0;

diff --git a/src/parser_bison.y b/src/parser_bison.y
index bf43608669cd1d7f49ab171caec5db539b64129e..dcabb464123f891c80e54101c75b758aebc7176b 100644 (file)
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -425,15 +425,12 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %type <cmd>                    base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
 %destructor { cmd_free($$); }  base_cmd add_cmd replace_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd describe_cmd
 
-%type <handle>                 table_spec chain_spec chain_identifier ruleid_spec ruleset_spec
-%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier ruleid_spec ruleset_spec
+%type <handle>                 table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
+%destructor { handle_free(&$$); } table_spec chain_spec chain_identifier ruleid_spec handle_spec position_spec rule_position ruleset_spec
 %type <handle>                 set_spec set_identifier
 %destructor { handle_free(&$$); } set_spec set_identifier
 %type <val>                    family_spec family_spec_explicit chain_policy prio_spec
 
-%type <handle_spec>            handle_spec
-%type <position_spec>          position_spec
-
 %type <string>                 dev_spec
 %destructor { xfree($$); }     dev_spec
 
@@ -720,11 +717,11 @@ add_cmd                   :       TABLE           table_spec
                                close_scope(state);
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_CHAIN, &$2, &@$, $5);
                        }
-                       |       RULE            ruleid_spec     rule
+                       |       RULE            rule_position   rule
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &$2, &@$, $3);
                        }
-                       |       /* empty */     ruleid_spec     rule
+                       |       /* empty */     rule_position   rule
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &$1, &@$, $2);
                        }
@@ -779,7 +776,7 @@ create_cmd          :       TABLE           table_spec
                        }
                        ;
 
-insert_cmd             :       RULE            ruleid_spec     rule
+insert_cmd             :       RULE            rule_position   rule
                        {
                                $$ = cmd_alloc(CMD_INSERT, CMD_OBJ_RULE, &$2, &@$, $3);
                        }
@@ -1252,35 +1249,37 @@ set_identifier          :       identifier
                        }
                        ;
 
-handle_spec            :       /* empty */
+handle_spec            :       HANDLE          NUM
                        {
                                memset(&$$, 0, sizeof($$));
+                               $$.handle.location      = @$;
+                               $$.handle.id            = $2;
                        }
-                       |       HANDLE          NUM
+                       ;
+
+position_spec          :       POSITION        NUM
                        {
                                memset(&$$, 0, sizeof($$));
-                               $$.location     = @$;
-                               $$.id           = $2;
+                               $$.position.location    = @$;
+                               $$.position.id          = $2;
                        }
                        ;
 
-position_spec          :       /* empty */
+rule_position          :       chain_spec
                        {
-                               memset(&$$, 0, sizeof($$));
+                               $$ = $1;
                        }
-                       |       POSITION        NUM
+                       |       chain_spec      position_spec
                        {
-                               memset(&$$, 0, sizeof($$));
-                               $$.location     = @$;
-                               $$.id           = $2;
+                               handle_merge(&$1, &$2);
+                               $$ = $1;
                        }
                        ;
 
-ruleid_spec            :       chain_spec      handle_spec     position_spec
+ruleid_spec            :       chain_spec      handle_spec
                        {
-                               $$              = $1;
-                               $$.handle       = $2;
-                               $$.position     = $3;
+                               handle_merge(&$1, &$2);
+                               $$ = $1;
                        }
                        ;