On Tue, Jun 27, 2023 at 2:13 AM Alexander Zubkov <[email protected]> wrote:
>
> On Mon, Jun 26, 2023 at 5:54 PM Alexander Zubkov <[email protected]> wrote:
> >
> > On Mon, Jun 26, 2023 at 5:43 PM Ondrej Zajicek <[email protected]>
> > wrote:
> > >
> > > On Mon, Jun 26, 2023 at 03:24:47AM +0200, Alexander Zubkov wrote:
> > > > On Sat, Jun 24, 2023 at 3:16 PM Ondrej Zajicek <[email protected]>
> > > > wrote:
> > > > >
> > > > > On Sat, Jun 24, 2023 at 02:20:03AM +0200, Alexander Zubkov wrote:
> > > > > > > Yes, the original idea there was to add bytestring as a data
> > > > > > > type, make
> > > > > > > hex() a regular (filter) function instead of special function-like
> > > > > > > syntax, and add equivalent of 'expr' grammar term for other data
> > > > > > > types.
> > > > > > >
> > > > > >
> > > > > > I see. I think I can look into preparing a patch for that too.
> > > > > > But for such variant I would suggest using function names like
> > > > > > "from_hex/base64" instead of "hex/base64", or something including
> > > > > > bytestring reference: "bs_hex". Because the simple variants could be
> > > > > > misleading when used not only in the limited set of scopes.
> > > > > > they can be thought of converting to hex/base64 representation too.
> > > > > > Or they
> > > > > > could collide with "hex" function to convert from string to int,
> > > > > > which
> > > > > > someone would need to implement in the future.
> > > > >
> > > > > Yes, that is true.
> > > > >
> > > > > You can try it if you are brave enough to add new f_val type.
> > > >
> > > > Take a look at the patch, please. Waiting for the critics and
> > > > improvement suggestions.
> > >
> > > Hi
> > >
> > > It looks pretty good. First, could you split it to at least four patches?
> >
> > Sure. I'll provide split patches later.
> >
> > >
> > > 1) unrelated changes, like the newline-in-string-constant
> > > 2) preparatory changes (functions in lib/bytestring.c, change to
> > > BYTESTRING lexer)
> > > 3) adding bytestring type to filter code (including FI_FROM_HEX inst)
>
> Added patches up to this point. There are also some fixes and
> modification. For example, I noted that 'bytestring' symbol for the
> type name conflicts with lexer's BYTESTRING id. So I had to rename
> lexer's BYTESTRING to BYTETEXT (like it is done for strings).
> For the following patches it is better to decide the structure of the
> new *eval* functions.
>
> > > 4) change to parser related to f_eval_val(), bytestring nonterminal and
> > > so on.
Final patches to modify current f_eval_int() to generic approach. And
for nonterminal bytestring.
Again, waiting for comments/suggestions. Also feel free, of course, to
fix naming/etc when applying.
Next, I will be able to move forward with patches to the documentation.
> > >
> > > Some more comments:
> > >
> > > > It was needed to add another function like f_eval_int(), so I decided
> > > > to do some more generic approach and replaced all occurences of
> > > > f_eval_int() with it.
> > >
> > > That is good approach, although it would be probably better to call this
> > > function like cf_eval(), associated macro as cf_eval_val, and keep some
> > > inline functions like cf_eval_int(), cf_eval_bs() and so on.
> > >
> > > Or perhaps cf_eval() could return f_val as return value, and have
> > > shorthand functions like:
> > >
> > > static inline cf_eval_int(..) { return cf_eval(.., T_INT).i; }
>
> I actually tried first to return the struct instead of modifying it by
> a reference. But for that we need to have "struct f_val" known in
> filter/filter.h, which is defined in filter/data.h. But that causes
> some circular dependencies problem. I didn't dig deep into it, but
> maybe it is possible to solve the conflict in a clean way.
Looked at it again. It seems safe to include filter/data.h into
filter/filter.h. I probably had problems when trying to incude it
somewhere else, until it all finalized into filter.h
>
> > >
> > > I will give more comments later.
> > >
> > > --
> > > Elen sila lumenn' omentielvo
> > >
> > > Ondrej 'Santiago' Zajicek (email: [email protected])
> > > OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> > > "To err is human -- to blame it on a computer is even more so."
From 63f9dcc924599915a93cb71f1f9e9cbb8ebd087b Mon Sep 17 00:00:00 2001
From: Alexander Zubkov <[email protected]>
Date: Thu, 29 Jun 2023 13:00:12 +0200
Subject: [PATCH] Conf: use more generic approach for intra-config expression
evaluation
Replace f_eval_int() function with a type-generic variant: cf_eval().
And implement similar fuction: cf_eval_int() via inline call to cf_eval().
---
conf/confbase.Y | 4 ++--
filter/config.Y | 8 ++++----
filter/filter.c | 22 +++++++---------------
filter/filter.h | 8 +++++---
nest/config.Y | 2 +-
5 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/conf/confbase.Y b/conf/confbase.Y
index da750d38..e109ddf5 100644
--- a/conf/confbase.Y
+++ b/conf/confbase.Y
@@ -155,14 +155,14 @@ conf: definition ;
definition:
DEFINE symbol '=' term ';' {
struct f_val *val = cfg_allocz(sizeof(struct f_val));
- if (f_eval(f_linearize($4, 1), cfg_mem, val) > F_RETURN) cf_error("Runtime error");
+ *val = cf_eval($4, T_VOID);
cf_define_symbol($2, SYM_CONSTANT | val->type, val, val);
}
;
expr:
NUM
- | '(' term ')' { $$ = f_eval_int(f_linearize($2, 1)); }
+ | '(' term ')' { $$ = cf_eval_int($2); }
| CF_SYM_KNOWN {
if ($1->class != (SYM_CONSTANT | T_INT)) cf_error("Number constant expected");
$$ = SYM_VAL($1).i; }
diff --git a/filter/config.Y b/filter/config.Y
index 7c52847b..960b173a 100644
--- a/filter/config.Y
+++ b/filter/config.Y
@@ -360,7 +360,7 @@ filter_def:
conf: filter_eval ;
filter_eval:
- EVAL term { f_eval_int(f_linearize($2, 1)); }
+ EVAL term { cf_eval_int($2); }
;
conf: custom_attr ;
@@ -575,7 +575,7 @@ set_atom:
| VPN_RD { $$.type = T_RD; $$.val.ec = $1; }
| ENUM { $$.type = pair_a($1); $$.val.i = pair_b($1); }
| '(' term ')' {
- if (f_eval(f_linearize($2, 1), cfg_mem, &($$)) > F_RETURN) cf_error("Runtime error");
+ $$ = cf_eval($2, T_VOID);
if (!f_valid_set_type($$.type)) cf_error("Set-incompatible type");
}
| CF_SYM_KNOWN {
@@ -587,13 +587,13 @@ set_atom:
switch_atom:
NUM { $$.type = T_INT; $$.val.i = $1; }
- | '(' term ')' { $$.type = T_INT; $$.val.i = f_eval_int(f_linearize($2, 1)); }
+ | '(' term ')' { $$ = cf_eval($2, T_INT); }
| fipa { $$ = $1; }
| ENUM { $$.type = pair_a($1); $$.val.i = pair_b($1); }
;
cnum:
- term { $$ = f_eval_int(f_linearize($1, 1)); }
+ term { $$ = cf_eval_int($1); }
pair_item:
'(' cnum ',' cnum ')' { $$ = f_new_pair_item($2, $2, $4, $4); }
diff --git a/filter/filter.c b/filter/filter.c
index 20a380dc..d080cadc 100644
--- a/filter/filter.c
+++ b/filter/filter.c
@@ -373,30 +373,22 @@ f_eval(const struct f_line *expr, struct linpool *tmp_pool, struct f_val *pres)
}
/*
- * f_eval_int - get an integer value of a term
+ * cf_eval - evaluate a value of a term and check its type
* Called internally from the config parser, uses its internal memory pool
* for allocations. Do not call in other cases.
*/
-uint
-f_eval_int(const struct f_line *expr)
+struct f_val
+cf_eval(const struct f_inst *inst, int type)
{
- /* Called independently in parse-time to eval expressions */
- filter_state = (struct filter_state) {
- .stack = &filter_stack,
- .pool = cfg_mem,
- };
-
struct f_val val;
- LOG_BUFFER_INIT(filter_state.buf);
-
- if (interpret(&filter_state, expr, &val) > F_RETURN)
+ if (f_eval(f_linearize(inst, 1), cfg_mem, &val) > F_RETURN)
cf_error("Runtime error while evaluating expression; see log for details");
- if (val.type != T_INT)
- cf_error("Integer expression expected");
+ if (type != T_VOID && val.type != type)
+ cf_error("Expression of type %s expected", f_type_name(type));
- return val.val.i;
+ return val;
}
/*
diff --git a/filter/filter.h b/filter/filter.h
index 26c1037b..91de696c 100644
--- a/filter/filter.h
+++ b/filter/filter.h
@@ -15,6 +15,7 @@
#include "lib/macro.h"
#include "nest/route.h"
#include "nest/attrs.h"
+#include "filter/data.h"
/* Possible return values of filter execution */
enum filter_return {
@@ -40,9 +41,8 @@ static inline const char *filter_return_str(const enum filter_return fret) {
}
}
-struct f_val;
-
/* The filter encapsulating structure to be pointed-to from outside */
+struct f_inst;
struct f_line;
struct filter {
struct symbol *sym;
@@ -53,9 +53,11 @@ struct rte;
enum filter_return f_run(const struct filter *filter, struct rte **rte, struct linpool *tmp_pool, int flags);
enum filter_return f_eval_rte(const struct f_line *expr, struct rte **rte, struct linpool *tmp_pool);
-uint f_eval_int(const struct f_line *expr);
enum filter_return f_eval_buf(const struct f_line *expr, struct linpool *tmp_pool, buffer *buf);
+struct f_val cf_eval(const struct f_inst *inst, int type);
+static inline uint cf_eval_int(const struct f_inst *inst) { return cf_eval(inst, T_INT).val.i; };
+
const char *filter_name(const struct filter *filter);
int filter_same(const struct filter *new, const struct filter *old);
int f_same(const struct f_line *f1, const struct f_line *f2);
diff --git a/nest/config.Y b/nest/config.Y
index 9197f985..610b1c01 100644
--- a/nest/config.Y
+++ b/nest/config.Y
@@ -167,7 +167,7 @@ rtrid:
idval:
NUM { $$ = $1; }
- | '(' term ')' { $$ = f_eval_int(f_linearize($2, 1)); }
+ | '(' term ')' { $$ = cf_eval_int($2); }
| IP4 { $$ = ip4_to_u32($1); }
| CF_SYM_KNOWN {
if ($1->class == (SYM_CONSTANT | T_INT) || $1->class == (SYM_CONSTANT | T_QUAD))
--
2.41.0
From fdb7c222bf234ab6cc9d1757642ac336fb39bbd8 Mon Sep 17 00:00:00 2001
From: Alexander Zubkov <[email protected]>
Date: Thu, 29 Jun 2023 13:51:01 +0200
Subject: [PATCH] Conf: use nonterminal bytestring instead of BYTETEXT
Nonterminal bytestring allows to provide expressions to be evaluated in places
where BYTETEXT is used now: passwords, radv custom option.
---
conf/confbase.Y | 40 ++++++++++++++++++++++++++++++++++++++++
nest/config.Y | 5 +----
proto/radv/config.Y | 4 ++--
3 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/conf/confbase.Y b/conf/confbase.Y
index e109ddf5..5006c185 100644
--- a/conf/confbase.Y
+++ b/conf/confbase.Y
@@ -117,9 +117,12 @@ CF_DECLS
%type <mls> label_stack_start label_stack
%type <t> text opttext
+%type <bs> bytestring
%type <s> symbol
%type <kw> kw_sym
+%type <x> bytestring_inst
+
%nonassoc PREFIX_DUMMY
%left AND OR
%nonassoc '=' '<' '>' '~' GEQ LEQ NEQ NMA PO PC
@@ -395,6 +398,43 @@ opttext:
| /* empty */ { $$ = NULL; }
;
+bytestring:
+ BYTETEXT
+ | TEXT {
+ int len = strlen($1);
+ struct bytestring *bs = cfg_allocz(sizeof(*bs) + len);
+ $$ = bs;
+ bs->length = len;
+ memcpy(bs->data, $1, len);
+ }
+ | bytestring_inst {
+ struct f_val val = cf_eval($1, T_VOID);
+
+ switch (val.type) {
+ case T_BYTESTRING:
+ $$ = val.val.bs;
+ break;
+
+ case T_STRING:
+ int len = strlen(val.val.s);
+ struct bytestring *bs = cfg_allocz(sizeof(*bs) + len);
+ $$ = bs;
+ bs->length = len;
+ memcpy(bs->data, val.val.s, len);
+ break;
+
+ default:
+ cf_error("Bytestring value is expected");
+ }
+ }
+ ;
+
+bytestring_inst:
+ symbol_value
+ | term_bs
+ | '(' term ')' { $$ = $2; }
+ ;
+
CF_CODE
diff --git a/nest/config.Y b/nest/config.Y
index 610b1c01..2fdbb991 100644
--- a/nest/config.Y
+++ b/nest/config.Y
@@ -546,10 +546,7 @@ password_item:
pass_key: PASSWORD | KEY;
-password_item_begin:
- pass_key text { init_password_list(); init_password($2, strlen($2), password_id++); }
- | pass_key BYTETEXT { init_password_list(); init_password($2->data, $2->length, password_id++); }
-;
+password_item_begin: pass_key bytestring { init_password_list(); init_password($2->data, $2->length, password_id++); };
password_item_params:
/* empty */ { }
diff --git a/proto/radv/config.Y b/proto/radv/config.Y
index 9653cd7b..9c5e1761 100644
--- a/proto/radv/config.Y
+++ b/proto/radv/config.Y
@@ -73,7 +73,7 @@ radv_proto_item:
| PREFIX radv_prefix { add_tail(&RADV_CFG->pref_list, NODE this_radv_prefix); }
| RDNSS { init_list(&radv_dns_list); } radv_rdnss { add_tail_list(&RADV_CFG->rdnss_list, &radv_dns_list); }
| DNSSL { init_list(&radv_dns_list); } radv_dnssl { add_tail_list(&RADV_CFG->dnssl_list, &radv_dns_list); }
- | CUSTOM OPTION TYPE expr VALUE BYTETEXT { radv_add_to_custom_list(&RADV_CFG->custom_list, $4, $6); }
+ | CUSTOM OPTION TYPE expr VALUE bytestring { radv_add_to_custom_list(&RADV_CFG->custom_list, $4, $6); }
| TRIGGER net_ip6 { RADV_CFG->trigger = $2; }
| PROPAGATE ROUTES bool { RADV_CFG->propagate_routes = $3; }
;
@@ -138,7 +138,7 @@ radv_iface_item:
| PREFIX radv_prefix { add_tail(&RADV_IFACE->pref_list, NODE this_radv_prefix); }
| RDNSS { init_list(&radv_dns_list); } radv_rdnss { add_tail_list(&RADV_IFACE->rdnss_list, &radv_dns_list); }
| DNSSL { init_list(&radv_dns_list); } radv_dnssl { add_tail_list(&RADV_IFACE->dnssl_list, &radv_dns_list); }
- | CUSTOM OPTION TYPE expr VALUE BYTETEXT { radv_add_to_custom_list(&RADV_IFACE->custom_list, $4, $6); }
+ | CUSTOM OPTION TYPE expr VALUE bytestring { radv_add_to_custom_list(&RADV_IFACE->custom_list, $4, $6); }
| RDNSS LOCAL bool { RADV_IFACE->rdnss_local = $3; }
| DNSSL LOCAL bool { RADV_IFACE->dnssl_local = $3; }
| CUSTOM OPTION LOCAL bool { RADV_IFACE->custom_local = $4; }
--
2.41.0
