Re: [PATCH nft 4/5,v2] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux