Hi Alexander,
On Fri, Oct 27, 2023 at 02:12:10PM +0000, Stephan, Alexander wrote:
> > BTW, please check if this works in default-server directives.
>
> struct srv_pp_tlv_list {
> struct list list;
> struct list fmt;
> unsigned char type;
> };
>
> To allow for use with the default server, I adjusted srv_settings_cpy
> accordingly such that the server TLV entries (see above) are deep copied.
> It works, but there is an edge case with the struct logformat_node that is
> contained in such a TLV struct. default-server directives
> Its member expr has the type of a void pointer, therefore I cannot directly
> copy it. Alternatively, if I would reuse the memory by simply copying the
> pointer, a double free will likely happen in srv_drop (I always free the TLV
> list in it, alongside the logformat node list).
> Besides, the servers created from the default-backend should operate
> independently, so this is not an option, I guess.
Definitely. From what I remember about what we do for other such
expressions that are inherited from defaults, we use a trick: we only
save the expression as a string during parsing, that's the same that
is kept on the default server. Thus the new server just does an strdup()
of that log-format expression that's just a string. And only later we
resolve it. That's not the best example but the first I just found,
please have a look at uniqueid_format_string. It's handled exactly
like this and resolved in check_config_validity().
> > You may want to have a look at how the "sni" keyword which also supports an
> > expression is handled, as I don't recall the exact details.
> > Maybe in your case we don't need the expression but the log-format instead
> > and it's not an issue, I just don't remember the details without having a
> > deeper look to be honest.
>
> Maybe let's just use a sample expression as its usage is more straight
> forward, basically similar to the sni option? Or do you have any other ideas
> on how to tackle the issue I described above?
Parsing the log-format string late definitely is the best option, and not
too cumbersome. You can even still report the line number etc from the
"server" struct to indicate in the parsing error if any, since everything
is on the same line, so there's really no problem with parsing late.
> Disabling the default-server support could be another solution. I am very
> interested in your opinion on this.
I'd really avoid disabling default-server as much as possible, or it
will start to be quite difficult to configure given that other related
options are accepted there. I tend to consider that the effort needed
by users to work around our limited designs often outweighs the efforts
we should have involved to make it smoother for them, so until we're
certain it's not possible it's worth trying ;-)
Thanks!
Willy
