Re: [nft PATCH v2 2/2] src: get rid of printf

[Phil Sutter <phil@nwl.cc>]

Hi,

On Thu, Sep 21, 2017 at 05:51:12PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 21, 2017 at 05:43:09PM +0200, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > With the proposed implementation of nft_print(), this will be
> > > problematic: nft_run_cmd_from_buffer() waits for the command to finish
> > > before printing all the output at once. This obviously breaks monitor
> > > which runs endlessly.
> > > 
> > > Maybe we should reference nft_print via a function pointer in
> > > output_ctx? This would allow to configure a different nft_print
> > > implementation which flushes the buffer to stdout immediately.
> > > 
> > > What do you think?
> > 
> > I wondered the same thing.
> > 
> > I have following issue: When using new typeof keyword then listing
> > a set definition should output something like
> > 
> > set foo { type typeof(meta iifname) }
> > 
> > Because the kernel has no notion of 'meta iifname', we have to
> > store this information in the kernel so we can read it back during
> > delinearization.
> > 
> > One way to do this would be to store the 'meta iifname' string in the
> > sets userdata.
> > 
> > For that, expr_print() would have to be able to print to a buffer
> > (or we would need an expr_snprintf or something similar) to extract
> > the convert struct *expr back to its original name.
> > 
> > The other solution would be to stash this in the expression
> > during parsing but that seems weird as we need to be able to do such
> > conversion anyway when printing the ruleset, so we merely need
> > to make this accessible outside of plain printf() to stdout.
> 
> Probably the underlying problem is that the monitor code is that not
> delinearing, ie. transforming from netlink to abstract syntax tree
> (ast) before printing?

No, we are talking about different issues (but with a potential common
solution):

Florian searches for a way to "convert" an expression from it's binary
form (a struct expr) into a human readable string (here: "meta
iifname"). We have this "converter" already in very limited form, namely
expr_print(). Eric's patch changes that function to make use of
nft_print(), which effectively allows to define a custom buffer to print
into.

Florian, did I get this right?

Cheers, Phil