On Fri, Jul 27, 2018 at 04:21:46PM +0200, Máté Eckl wrote:
> On Mon, Jul 16, 2018 at 09:58:44AM +0200, Máté Eckl wrote:
> > On Tue, Jul 10, 2018 at 12:10:22PM +0200, Pablo Neira Ayuso wrote:
> > > > diff --git a/src/parser_bison.y b/src/parser_bison.y
> > > > index 98bfeba..2b7d7cc 100644
> > > > --- a/src/parser_bison.y
> > > > +++ b/src/parser_bison.y
> > > > @@ -182,6 +182,8 @@ int nft_lex(void *, void *, void *);
> > > > %token AT "@"
> > > > %token VMAP "vmap"
> > > >
> > > > +%token PLUS "+"
> > > > +
> > > > %token INCLUDE "include"
> > > > %token DEFINE "define"
> > > > %token REDEFINE "redefine"
> > > > @@ -522,6 +524,7 @@ int nft_lex(void *, void *, void *);
> > > > %type <handle> set_spec setid_spec set_identifier flowtable_identifier obj_spec objid_spec obj_identifier
> > > > %destructor { handle_free(&$$); } set_spec setid_spec set_identifier obj_spec objid_spec obj_identifier
> > > > %type <val> family_spec family_spec_explicit chain_policy prio_spec
> > > > +%type <string> str_prio_spec
> > > >
> > > > %type <string> dev_spec quota_unit
> > > > %destructor { xfree($$); } dev_spec quota_unit
> > > > @@ -1633,7 +1636,7 @@ flowtable_block_alloc : /* empty */
> > > > flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
> > > > | flowtable_block common_block
> > > > | flowtable_block stmt_separator
> > > > - | flowtable_block HOOK STRING PRIORITY prio_spec stmt_separator
> > > > + | flowtable_block HOOK STRING PRIORITY str_prio_spec stmt_separator
> > > > {
> > > > $$->hookstr = chain_hookname_lookup($3);
> > > > if ($$->hookstr == NULL) {
> > > > @@ -1644,7 +1647,7 @@ flowtable_block : /* empty */ { $$ = $<flowtable>-1; }
> > > > }
> > > > xfree($3);
> > > >
> > > > - $$->priority = $5;
> > > > + $$->priostr = $5;
> > > > }
> > > > | flowtable_block DEVICES '=' flowtable_expr stmt_separator
> > > > {
> > > > @@ -1766,7 +1769,7 @@ type_identifier : STRING { $$ = $1; }
> > > > | CLASSID { $$ = xstrdup("classid"); }
> > > > ;
> > > >
> > > > -hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
> > > > +hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY str_prio_spec
> > > > {
> > > > const char *chain_type = chain_type_name_lookup($2);
> > > >
> > > > @@ -1789,13 +1792,34 @@ hook_spec : TYPE STRING HOOK STRING dev_spec PRIORITY prio_spec
> > > > xfree($4);
> > > >
> > > > $<chain>0->dev = $5;
> > > > - $<chain>0->priority = $7;
> > > > + $<chain>0->priostr = $7;
> > > > $<chain>0->flags |= CHAIN_F_BASECHAIN;
> > > > }
> > > > ;
> > > >
> > > > -prio_spec : NUM { $$ = $1; }
> > > > - | DASH NUM { $$ = -$2; }
> > > > +str_prio_spec : prio_spec
> > > > + {
> > > > + char buf[STD_PRIO_BUFSIZE];
> > > > + snprintf(buf, STD_PRIO_BUFSIZE, "%d", (int)$1);
> > > > + $$ = xstrdup(buf);
> > > > + }
> > > > + | STRING { $$ = xstrdup($1); }
> > > > + | STRING PLUS NUM
> > > > + {
> > > > + char buf[STD_PRIO_BUFSIZE];
> > > > + snprintf(buf, STD_PRIO_BUFSIZE, "%s+%d",$1, (int)$3);
> > >
> > > Could you store the string plus offset instead of building this
> > > string that you need to parse again from the evaluation phase?
> > >
> > > Probably you could reuse the existing priority integer field, then, if
> > > the label is non-NULL, then it means the priority integer becomes an
> > > offset.
> >
> > I thought about different possibilities to do this, and I think the diff below
> > does this with the less possible code duplication (it's only the parser, the
> > other components would not be duplicated). And for now it does not even work for
> > some obscure reason. (The chain pointer is not tha same at the evaluation phase
> > as in the parser, so the values are in the bad place...)
> > Of course the evaluation is simpler.
> >
> > I personally prefer my former solution as it is more general and results in less
> > code duplication so I would stay with that.
> > What do you think?
>
> Do you want me to go on with this Pablo? I can make this work, but I don't think
> this is better than what I've already done.
If you're refering to the string parsing in the evaluation phase that
this patch is doing, I would indeed prefer an approach that which
doesn't need any tinkering with str*() functions to do late
tokenization from there.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
