Hi,

On Mon, Oct 29, 2018 at 01:43:00PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Oct 29, 2018 at 01:29:32PM +0100, Phil Sutter wrote:
> > On Mon, Oct 29, 2018 at 12:33:39PM +0100, Pablo Neira Ayuso wrote:
[...]
> > > 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? :-)

Will do!

Thanks, Phil

Reply via email to