[PATCH] adding custom options in radv protocol, strict ipv6 regex

[Ondrej Zajicek]

On Mon, Jun 26, 2023 at 03:24:47AM +0200, Alexander Zubkov wrote:
> On Sat, Jun 24, 2023 at 3:16 PM Ondrej Zajicek <santiago at crfreenet.org> 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?

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)
4) change to parser related to f_eval_val(), bytestring nonterminal and so on.

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 will give more comments later.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at 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."