Hi,
While waiting for the fate of the previous patches, I was thinking
about that thing about using keywords as symbols. So here is another
longread. :)
Now it is not possible to mix a keyword and a keyword as a symbol
together. Here is what I mean. With current master bird if I use
config:
protocol device {}
template bgp { local role peer; }
function role() { int role = 1; return role; }
It parses ok, because "role" keyword is used before it is converted
into a symbol. But if it appears as a symbol before:
protocol device {}
function role() { int role = 1; return role; }
template bgp { local role peer; }
It returns an error:
bird: t5.conf:3:22 IP address constant expected
Because "role" is converted to a symbol and it is not possible to use
it as a keyword anymore. I thought if something can be done about that
and I probably have a solution. See the attached patch. It maybe not
the best design, just to show the idea. I change the order of
detecting symbol and keyword in the lexer, so it always returns a
keyword for a keyword, and it is converted into a symbol only in the
parser when it is used as a symbol. As we can hit such symbol keyword
many times, I check if it has already been defined and create a new
one if needed. Then I replace mentions of CF_SYM_KNOWN with
symbol_known, which can be CF_SYM_KNOWN or kw_sym. So, with this patch
applied, both configs are successfully parsed and work well.
Also, I think that the current realization in bird relies on the fact
that lexer would not have symbols parsed in advance, i.e. that further
mentions of the keyword should return CF_SYM_KNOWN. But if it is not
the case and the lexer parses forward for some reason (before the
parser creates the symbol for the keyword) it would not return
CF_SYM_KNOWN. I don't know the internals of the lexer and parser much,
maybe it is impossible situation (parser does not backtrack, etc.).
And there is no need to worry here.
Some additional notes.
My previous remark regarding the missing "|" in "kw_sym: MIN MAX"
seems to be correct, because current bird master fails on such config:
protocol device {}
function min() { return 0; }
With the error:
bird: t4.conf:2:13 syntax error, unexpected '(', expecting MAX
And also there seems to be a bug with recently added syntax for
variable definitions. When they are defined without initializing. This
config is successfully loaded:
protocol device {}
function role() { int role; role = 1; return role; }
But if I try to call "eval role()" from cli, I get an error: "runtime
error", and the daemon logs this:
bird: filters, line 2: Argument 1 of VAR_INIT must be of type int, got type void
So it looks like it tries to initialize it with the void value and
fails to do it. I haven't tracked the source of this bug.
On Wed, Jun 14, 2023 at 12:40 AM Alexander Zubkov <gr...@qrator.net> wrote:
>
> Hi,
>
> Please look at these patches:
>
> bytestring-hex-prefix.patch - syntax with "hex:" prefix
> I allowed mixed colons with no-divider there, so hex:12:345678:90 is
> allowed. As there is a distinguishing prefix here, this should not be
> a problem.
> Empty bytestrings are allowed too: "hex:"
>
> bytestring-hex-function.patch - function-like hex("...") syntax (on
> top of the other patch)
> It wasn't too complex, but you might have wanted to do it some other
> way. I think this should be quite good too, the only problem with it
> is inability to mix "hex" symbol with hex("...") bytestrings.
> I parse string in two steps. First to know the length of a binary
> output. Current hex realization does the same anyway. I made a
> separate function for it to reuse it for "classic" bytestrings and for
> function-like syntax.
> It ignores all non-alphanumeric symbols and requires that each byte
> symbols go in pairs without delimiters.
> I added a HEX keyword and added it to kw_sym, but this solution does
> not allow to mix "hex" symbol and hex("...") bytestring in the same
> config.
> There probably was a mistake in nest/config.Y with missing "|" here:
> "kw_sym: MIN MAX ;"
> There is a "TODO" in the heading of strtobin.c, don't know what is
> your policy regarding that. The code is still based on the current
> BYTESTRING parser.
> I also enabled TEXT literals to be multiline. Didn't know it was not
> allowed already, so I think it should not break things, but allows to
> be more flexible with binary strings.
>
> On Tue, Jun 13, 2023 at 6:37 PM Ondrej Zajicek <santi...@crfreenet.org> wrote:
> >
> > On Tue, Jun 13, 2023 at 05:34:18PM +0200, Alexander Zubkov wrote:
> > > On Tue, Jun 13, 2023, 16:07 Ondrej Zajicek <santi...@crfreenet.org> wrote:
> > >
> > > > We agreed on keeping existing format for suffiently long hex
> > > > bytestrings,
> > > > using hex: prefix for bytestrings of any length, and adding hex("...") /
> > > > base64("...") as ordinary expressions.
> > >
> > > Hi,
> > >
> > > So full house. :) I can try to implement it. Do you need a hand?
> >
> > If you implement hex: prefix, we could easily merge that. The
> > hex("...") / base64("...") is not priority and would require some
> > nontrivial changes to be done properly.
> >
> > --
> > Elen sila lumenn' omentielvo
> >
> > Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
> > OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> > "To err is human -- to blame it on a computer is even more so."
commit fcc0c6c8a7d16921683888b96c1db2e35e381c72
Author: Alexander Zubkov <gr...@qrator.net>
Date: Thu Jun 15 03:04:02 2023 +0200
Conf: allow using keywords and symbols with the same name simulatneousely
diff --git a/conf/cf-lex.l b/conf/cf-lex.l
index 9555949d..9672a2fe 100644
--- a/conf/cf-lex.l
+++ b/conf/cf-lex.l
@@ -603,7 +603,17 @@ cf_new_symbol(const byte *c)
struct symbol *
cf_symbol_from_keyword(const struct keyword *kw)
-{ return cf_new_symbol(kw->name); }
+{
+ struct symbol *sym = cf_known_symbol_from_keyword(kw);
+ if (!sym)
+ sym = cf_new_symbol(kw->name);
+
+ return sym;
+}
+
+struct symbol *
+cf_known_symbol_from_keyword(const struct keyword *kw)
+{ return cf_find_local_symbol(new_config, conf_this_scope, kw->name); }
/**
* cf_find_local_symbol - find a symbol by name
@@ -695,15 +705,6 @@ cf_default_name(char *template, int *counter)
static enum yytokentype
cf_lex_symbol(const char *data)
{
- /* Have we defined such a symbol? */
- struct symbol *sym = cf_find_local_symbol(new_config, conf_this_scope, data);
-
- if (sym && (sym->class != SYM_VOID))
- {
- cf_lval.s = sym;
- return CF_SYM_KNOWN;
- }
-
/* Is it a keyword? */
struct keyword *k = HASH_FIND(kw_hash, KW, data);
if (k)
@@ -720,6 +721,15 @@ cf_lex_symbol(const char *data)
}
}
+ /* Have we defined such a symbol? */
+ struct symbol *sym = cf_find_local_symbol(new_config, conf_this_scope, data);
+
+ if (sym && (sym->class != SYM_VOID))
+ {
+ cf_lval.s = sym;
+ return CF_SYM_KNOWN;
+ }
+
/* OK, undefined symbol */
cf_lval.s = cf_new_symbol(data);
return CF_SYM_UNDEFINED;
diff --git a/conf/conf.h b/conf/conf.h
index d40f955e..27f6d7b4 100644
--- a/conf/conf.h
+++ b/conf/conf.h
@@ -193,6 +193,7 @@ static inline struct symbol *cf_find_symbol(const struct config *cfg, const byte
struct keyword;
struct symbol *cf_symbol_from_keyword(const struct keyword *kw);
+struct symbol *cf_known_symbol_from_keyword(const struct keyword *kw);
struct symbol *cf_get_symbol(const byte *c);
struct symbol *cf_default_name(char *template, int *counter);
diff --git a/conf/confbase.Y b/conf/confbase.Y
index 3e8f5807..2500a206 100644
--- a/conf/confbase.Y
+++ b/conf/confbase.Y
@@ -117,7 +117,7 @@ CF_DECLS
%type <mls> label_stack_start label_stack
%type <t> text opttext
-%type <s> symbol
+%type <s> symbol symbol_known
%type <kw> kw_sym
%nonassoc PREFIX_DUMMY
@@ -175,6 +175,13 @@ expr_us:
;
symbol: CF_SYM_UNDEFINED | CF_SYM_KNOWN | kw_sym { $$ = cf_symbol_from_keyword($1); } ;
+symbol_known: CF_SYM_KNOWN | kw_sym {
+ struct symbol *sym = cf_known_symbol_from_keyword($1);
+ if (!sym)
+ cf_error("No known symbol for keyword");
+ $$ = sym;
+ }
+ ;
/* Switches */
@@ -193,7 +200,7 @@ bool:
ipa:
IP4 { $$ = ipa_from_ip4($1); }
| IP6 { $$ = ipa_from_ip6($1); }
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class != (SYM_CONSTANT | T_IP)) cf_error("IP address constant expected");
$$ = SYM_VAL($1).ip;
}
@@ -308,7 +315,7 @@ net_:
net_ip4:
net_ip4_
- | CF_SYM_KNOWN {
+ | symbol_known {
if (($1->class != (SYM_CONSTANT | T_NET)) || (SYM_VAL($1).net->type != NET_IP4))
cf_error("IPv4 network constant expected");
$$ = * SYM_VAL($1).net;
@@ -317,7 +324,7 @@ net_ip4:
net_ip6:
net_ip6_
- | CF_SYM_KNOWN {
+ | symbol_known {
if (($1->class != (SYM_CONSTANT | T_NET)) || (SYM_VAL($1).net->type != NET_IP6))
cf_error("IPv6 network constant expected");
$$ = * SYM_VAL($1).net;
@@ -326,7 +333,7 @@ net_ip6:
net_ip:
net_ip_
- | CF_SYM_KNOWN {
+ | symbol_known {
if (($1->class != (SYM_CONSTANT | T_NET)) || !net_is_ip(SYM_VAL($1).net))
cf_error("IP network constant expected");
$$ = * SYM_VAL($1).net;
@@ -335,7 +342,7 @@ net_ip:
net_any:
net_
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class != (SYM_CONSTANT | T_NET))
cf_error("Network constant expected");
$$ = (net_addr *) SYM_VAL($1).net; /* Avoid const warning */
@@ -347,7 +354,7 @@ net_or_ipa:
| net_ip6_
| IP4 { net_fill_ip4(&($$), $1, IP4_MAX_PREFIX_LENGTH); }
| IP6 { net_fill_ip6(&($$), $1, IP6_MAX_PREFIX_LENGTH); }
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class == (SYM_CONSTANT | T_IP))
net_fill_ip_host(&($$), SYM_VAL($1).ip);
else if (($1->class == (SYM_CONSTANT | T_NET)) && net_is_ip(SYM_VAL($1).net))
@@ -384,7 +391,7 @@ time:
text:
TEXT
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class != (SYM_CONSTANT | T_STRING)) cf_error("String constant expected");
$$ = SYM_VAL($1).s;
}
diff --git a/filter/config.Y b/filter/config.Y
index a1e5e9f1..97c9f447 100644
--- a/filter/config.Y
+++ b/filter/config.Y
@@ -367,7 +367,7 @@ custom_attr: ATTRIBUTE type symbol ';' {
conf: bt_test_suite ;
bt_test_suite:
- BT_TEST_SUITE '(' CF_SYM_KNOWN ',' text ')' {
+ BT_TEST_SUITE '(' symbol_known ',' text ')' {
cf_assert_symbol($3, SYM_FUNCTION);
struct f_bt_test_suite *t = cfg_allocz(sizeof(struct f_bt_test_suite));
t->fn = $3->function;
@@ -380,7 +380,7 @@ bt_test_suite:
conf: bt_test_same ;
bt_test_same:
- BT_TEST_SAME '(' CF_SYM_KNOWN ',' CF_SYM_KNOWN ',' NUM ')' {
+ BT_TEST_SAME '(' symbol_known ',' symbol_known ',' NUM ')' {
cf_assert_symbol($3, SYM_FUNCTION);
cf_assert_symbol($5, SYM_FUNCTION);
struct f_bt_test_suite *t = cfg_allocz(sizeof(struct f_bt_test_suite));
@@ -461,7 +461,7 @@ function_vars:
filter_body: function_body ;
filter:
- CF_SYM_KNOWN {
+ symbol_known {
cf_assert_symbol($1, SYM_FILTER);
$$ = $1->filter;
}
@@ -574,7 +574,7 @@ set_atom:
if (f_eval(f_linearize($2, 1), cfg_mem, &($$)) > F_RETURN) cf_error("Runtime error");
if (!f_valid_set_type($$.type)) cf_error("Set-incompatible type");
}
- | CF_SYM_KNOWN {
+ | symbol_known {
cf_assert_symbol($1, SYM_CONSTANT);
if (!f_valid_set_type(SYM_TYPE($1))) cf_error("%s: set-incompatible type", $1->name);
$$ = *$1->val;
@@ -745,7 +745,7 @@ var_list: /* EMPTY */ { $$ = NULL; }
| var_list ',' term { $$ = $3; $$->next = $1; }
function_call:
- CF_SYM_KNOWN '(' var_list ')'
+ symbol_known '(' var_list ')'
{
if ($1->class != SYM_FUNCTION)
cf_error("You can't call something which is not a function. Really.");
@@ -764,7 +764,7 @@ function_call:
}
;
-symbol_value: CF_SYM_KNOWN
+symbol_value: symbol_known
{
switch ($1->class) {
case SYM_CONSTANT_RANGE:
@@ -900,7 +900,7 @@ var:
for_var:
type symbol { $$ = cf_define_symbol($2, SYM_VARIABLE | $1, offset, f_new_var(sym_->scope)); }
- | CF_SYM_KNOWN { $$ = $1; cf_assert_symbol($1, SYM_VARIABLE); }
+ | symbol_known { $$ = $1; cf_assert_symbol($1, SYM_VARIABLE); }
;
cmd:
@@ -925,7 +925,7 @@ cmd:
$$ = f_new_inst(FI_FOR_INIT, $6, $3);
$$->next = f_new_inst(FI_FOR_NEXT, $3, $9);
}
- | CF_SYM_KNOWN '=' term ';' {
+ | symbol_known '=' term ';' {
switch ($1->class) {
case SYM_VARIABLE_RANGE:
$$ = f_new_inst(FI_VAR_SET, $3, $1);
@@ -990,7 +990,7 @@ get_cf_position:
};
lvalue:
- CF_SYM_KNOWN { cf_assert_symbol($1, SYM_VARIABLE); $$ = (struct f_lval) { .type = F_LVAL_VARIABLE, .sym = $1 }; }
+ symbol_known { cf_assert_symbol($1, SYM_VARIABLE); $$ = (struct f_lval) { .type = F_LVAL_VARIABLE, .sym = $1 }; }
| static_attr { $$ = (struct f_lval) { .type = F_LVAL_SA, .sa = $1 }; }
| dynamic_attr { $$ = (struct f_lval) { .type = F_LVAL_EA, .da = $1 }; };
diff --git a/nest/config.Y b/nest/config.Y
index c83c715b..8f0a3792 100644
--- a/nest/config.Y
+++ b/nest/config.Y
@@ -154,7 +154,7 @@ CF_ENUM_PX(T_ENUM_AF, AF_, AFI_, IPV4, IPV6)
CF_GRAMMAR
-kw_sym: MIN MAX ;
+kw_sym: MIN | MAX ;
/* Setting of router ID */
@@ -169,7 +169,7 @@ idval:
NUM { $$ = $1; }
| '(' term ')' { $$ = f_eval_int(f_linearize($2, 1)); }
| IP4 { $$ = ip4_to_u32($1); }
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class == (SYM_CONSTANT | T_INT) || $1->class == (SYM_CONSTANT | T_QUAD))
$$ = SYM_VAL($1).i;
else if (($1->class == (SYM_CONSTANT | T_IP)) && ipa_is_ip4(SYM_VAL($1).ip))
@@ -266,7 +266,7 @@ proto_name:
cf_define_symbol($1, this_proto->class, proto, this_proto);
this_proto->name = $1->name;
}
- | FROM CF_SYM_KNOWN {
+ | FROM symbol_known {
if (($2->class != SYM_TEMPLATE) && ($2->class != SYM_PROTO)) cf_error("Template or protocol name expected");
struct symbol *s = cf_default_name(this_proto->protocol->template, &this_proto->protocol->name_counter);
@@ -276,7 +276,7 @@ proto_name:
proto_copy_config(this_proto, $2->proto);
}
- | symbol FROM CF_SYM_KNOWN {
+ | symbol FROM symbol_known {
if (($3->class != SYM_TEMPLATE) && ($3->class != SYM_PROTO)) cf_error("Template or protocol name expected");
cf_define_symbol($1, this_proto->class, proto, this_proto);
@@ -346,7 +346,7 @@ channel_end:
proto_channel: channel_start channel_opt_list channel_end;
-rtable: CF_SYM_KNOWN { cf_assert_symbol($1, SYM_TABLE); $$ = $1->table; } ;
+rtable: symbol_known { cf_assert_symbol($1, SYM_TABLE); $$ = $1->table; } ;
imexport:
FILTER filter { $$ = $2; }
@@ -623,7 +623,7 @@ CF_CLI(SHOW PROTOCOLS ALL, proto_patt2, [<protocol> | \"<pattern>\"], [[Show rou
{ proto_apply_cmd($4, proto_cmd_show, 0, 1); } ;
optproto:
- CF_SYM_KNOWN { cf_assert_symbol($1, SYM_PROTO); $$ = $1; }
+ symbol_known { cf_assert_symbol($1, SYM_PROTO); $$ = $1; }
| /* empty */ { $$ = NULL; }
;
@@ -663,7 +663,7 @@ r_args:
$$->addr = $3;
$$->addr_mode = RSD_ADDR_IN;
}
-| r_args TABLE CF_SYM_KNOWN {
+| r_args TABLE symbol_known {
cf_assert_symbol($3, SYM_TABLE);
$$ = $1;
rt_show_add_table($$, $3->table->table);
@@ -708,7 +708,7 @@ r_args:
$$ = $1;
$$->filtered = 1;
}
- | r_args export_mode CF_SYM_KNOWN {
+ | r_args export_mode symbol_known {
cf_assert_symbol($3, SYM_PROTO);
struct proto_config *c = (struct proto_config *) $3->proto;
$$ = $1;
@@ -725,7 +725,7 @@ r_args:
$$->export_channel = $3;
$$->tables_defined_by = RSD_TDB_INDIRECT;
}
- | r_args PROTOCOL CF_SYM_KNOWN {
+ | r_args PROTOCOL symbol_known {
cf_assert_symbol($3, SYM_PROTO);
struct proto_config *c = (struct proto_config *) $3->proto;
$$ = $1;
@@ -764,7 +764,7 @@ r_args_for:
$$ = cfg_alloc(sizeof(net_addr_ip6_sadr));
net_fill_ip6_sadr($$, $1, IP6_MAX_PREFIX_LENGTH, $3, IP6_MAX_PREFIX_LENGTH);
}
- | CF_SYM_KNOWN {
+ | symbol_known {
if ($1->class == (SYM_CONSTANT | T_IP))
{
$$ = cfg_alloc(ipa_is_ip4(SYM_VAL($1).ip) ? sizeof(net_addr_ip4) : sizeof(net_addr_ip6));
@@ -815,7 +815,7 @@ channel_sym:
;
channel_arg:
- CF_SYM_KNOWN '.' channel_sym {
+ symbol_known '.' channel_sym {
cf_assert_symbol($1, SYM_PROTO);
struct proto *p = $1->proto->proto;
if (!p) cf_error("%s is not a protocol", $1->name);
@@ -914,13 +914,13 @@ CF_CLI(RESTRICT,,,[[Restrict current CLI session to safe commands]])
{ this_cli->restricted = 1; cli_msg(16, "Access restricted"); } ;
proto_patt:
- CF_SYM_KNOWN { cf_assert_symbol($1, SYM_PROTO); $$.ptr = $1; $$.patt = 0; }
+ symbol_known { cf_assert_symbol($1, SYM_PROTO); $$.ptr = $1; $$.patt = 0; }
| ALL { $$.ptr = NULL; $$.patt = 1; }
| TEXT { $$.ptr = $1; $$.patt = 1; }
;
proto_patt2:
- CF_SYM_KNOWN { cf_assert_symbol($1, SYM_PROTO); $$.ptr = $1; $$.patt = 0; }
+ symbol_known { cf_assert_symbol($1, SYM_PROTO); $$.ptr = $1; $$.patt = 0; }
| { $$.ptr = NULL; $$.patt = 1; }
| TEXT { $$.ptr = $1; $$.patt = 1; }
;
diff --git a/proto/ospf/config.Y b/proto/ospf/config.Y
index 4b7d5a36..c168649e 100644
--- a/proto/ospf/config.Y
+++ b/proto/ospf/config.Y
@@ -556,7 +556,7 @@ lsadb_args:
| lsadb_args LSID idval { $$ = $1; $$->lsid = $3; }
| lsadb_args SELF { $$ = $1; $$->router = SH_ROUTER_SELF; }
| lsadb_args ROUTER idval { $$ = $1; $$->router = $3; }
- | lsadb_args CF_SYM_KNOWN { cf_assert_symbol($2, SYM_PROTO); $$ = $1; $$->proto = (struct ospf_proto *) proto_get_named($2, &proto_ospf); }
+ | lsadb_args symbol_known { cf_assert_symbol($2, SYM_PROTO); $$ = $1; $$->proto = (struct ospf_proto *) proto_get_named($2, &proto_ospf); }
;
CF_CODE
