On Mon, Oct 29, 2018 at 01:29:32PM +0100, Phil Sutter wrote:
> Hi,
>
> On Mon, Oct 29, 2018 at 12:33:39PM +0100, Pablo Neira Ayuso wrote:
> > Add NFT_CTX_OUTPUT_JSON flag and display output in json format.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> [...]
> > diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> > index 8b7aee9af134..5a3562c3266c 100644
> > --- a/doc/libnftables.adoc
> > +++ b/doc/libnftables.adoc
> [...]
> > @@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_STATELESS::
> > If stateless output has been requested then stateful data is not printed. Stateful data refers to those objects that carry run-time data, eg. the *counter* statement holds packet and byte counter values, making it stateful.
> > NFT_CTX_OUTPUT_HANDLE::
> > Upon insertion into the ruleset, some elements are assigned a unique handle for identification purposes. For example, when deleting a table or chain, it may be identified either by name or handle. Rules on the other hand must be deleted by handle because there is no other way to uniquely identify them. These functions allow to control whether ruleset listings should include handles or not.
> > +NFT_CTX_OUTPUT_JSON::
> > + If enabled at compile-time, libnftables accepts input in JSON format and is able to print output in JSON format as well. See *libnftables-json*(5) for a description of the supported schema. These functions control JSON output format, input is auto-detected.
>
> s/These functions control/This flag controls/
Done. Thanks!
> [...]
> > diff --git a/src/libnftables.c b/src/libnftables.c
> > index 6dc1be3d5ef8..ff7a53d22ba4 100644
> > --- a/src/libnftables.c
> > +++ b/src/libnftables.c
> > @@ -352,22 +352,6 @@ void nft_ctx_output_set_echo(struct nft_ctx *ctx, bool val)
> > ctx->output.echo = val;
> > }
> >
> > -bool nft_ctx_output_get_json(struct nft_ctx *ctx)
> > -{
> > -#ifdef HAVE_LIBJANSSON
> > - return ctx->output.json;
> > -#else
> > - return false;
> > -#endif
> > -}
> > -
> > -void nft_ctx_output_set_json(struct nft_ctx *ctx, bool val)
> > -{
> > -#ifdef HAVE_LIBJANSSON
> > - ctx->output.json = val;
> > -#endif
> > -}
> > -
>
> In above functions, I guarded output.json setting by whether JSON
> support was built-in.
I'm going to do the same here but...
> [...]
> > diff --git a/src/main.c b/src/main.c
> > index 97b8746608a7..8ea07641734d 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -271,7 +271,7 @@ int main(int argc, char * const *argv)
> > nft_ctx_output_set_echo(nft, true);
> > break;
> > case OPT_JSON:
> > - nft_ctx_output_set_json(nft, true);
> > + output_flags |= NFT_CTX_OUTPUT_JSON;
> > break;
> > case OPT_INVALID:
> > exit(EXIT_FAILURE);
>
> Maybe we should do the same here? Otherwise if JSON wasn't enabled at
> compile-time, calling 'nft -j' leads to no output at all. (Not sure if
> silently falling back to standard output formatting is a better choice
> after all. :)
... I think failing here is json support is not built-in is the way to
go - instead of silently ignoring it.
Would you mind send a follow up patch to change this behaviour? :-)
Thanks!
