Hi,
Please give some feedback.
On Thu, Jun 15, 2023 at 3:57 AM Alexander Zubkov <gr...@qrator.net> wrote:
>
> 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."
