Re: [nft PATCH 2/6] monitor: Fix printing of set declarations
On Wed, Jul 26, 2017 at 01:18:03PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2017 at 07:09:34PM +0200, Phil Sutter wrote:
> > On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > > > diff --git a/tests/monitor/testcases/set-maps.t
> > > > b/tests/monitor/testcases/set-maps.t
> > > > index d94016beb0767..6ea36cb9d11d6 100644
> > > > --- a/tests/monitor/testcases/set-maps.t
> > > > +++ b/tests/monitor/testcases/set-maps.t
> > > > @@ -2,7 +2,7 @@
> > > > I add table ip t
> > > > O add table ip t
> > > > I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > > > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > > > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
> > >
> > > So the proposal is to remove the whitespace? I think it's more
> > > readable the way it is already.
> >
> > No, it is about the missing semicolon after 'flags interval' (and after
> > 'timeout' and 'gc-interval' options if present). The output of 'nft
> > monitor' is not syntactically correct without this.
> >
> > You are correct in that it removes the whitespace before the closing
> > brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
> > semicolon + whitespace.
> >
> > Is it clear now?
>
> Thanks for explaining, yes this clarifies. Please, add examples next
> time to your commit messages.
Yes, will do. And in that case I already did, please see v2 containing
the remaining patches (starting with message ID
20170725183944.9377-1-p...@nwl.cc).
> BTW, no need to update .t files after patch 4/6 to add the space after
> semicolon?
No, because I changed diff options along with the fix for NEWGEN message
to ignore all whitespace, not just at EOL.
Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [nft PATCH 2/6] monitor: Fix printing of set declarations
On Tue, Jul 25, 2017 at 07:09:34PM +0200, Phil Sutter wrote:
> On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > > diff --git a/tests/monitor/testcases/set-maps.t
> > > b/tests/monitor/testcases/set-maps.t
> > > index d94016beb0767..6ea36cb9d11d6 100644
> > > --- a/tests/monitor/testcases/set-maps.t
> > > +++ b/tests/monitor/testcases/set-maps.t
> > > @@ -2,7 +2,7 @@
> > > I add table ip t
> > > O add table ip t
> > > I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
> >
> > So the proposal is to remove the whitespace? I think it's more
> > readable the way it is already.
>
> No, it is about the missing semicolon after 'flags interval' (and after
> 'timeout' and 'gc-interval' options if present). The output of 'nft
> monitor' is not syntactically correct without this.
>
> You are correct in that it removes the whitespace before the closing
> brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
> semicolon + whitespace.
>
> Is it clear now?
Thanks for explaining, yes this clarifies. Please, add examples next
time to your commit messages.
BTW, no need to update .t files after patch 4/6 to add the space after
semicolon?
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [nft PATCH 2/6] monitor: Fix printing of set declarations
On Tue, Jul 25, 2017 at 05:57:41PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> > diff --git a/tests/monitor/testcases/set-maps.t
> > b/tests/monitor/testcases/set-maps.t
> > index d94016beb0767..6ea36cb9d11d6 100644
> > --- a/tests/monitor/testcases/set-maps.t
> > +++ b/tests/monitor/testcases/set-maps.t
> > @@ -2,7 +2,7 @@
> > I add table ip t
> > O add table ip t
> > I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> > -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> > +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
>
> So the proposal is to remove the whitespace? I think it's more
> readable the way it is already.
No, it is about the missing semicolon after 'flags interval' (and after
'timeout' and 'gc-interval' options if present). The output of 'nft
monitor' is not syntactically correct without this.
You are correct in that it removes the whitespace before the closing
brace, but patch 4 "fixes" that by making 'stmt_separator' consist of
semicolon + whitespace.
Is it clear now?
Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [nft PATCH 2/6] monitor: Fix printing of set declarations
On Tue, Jul 25, 2017 at 04:56:25PM +0200, Phil Sutter wrote:
> diff --git a/tests/monitor/testcases/set-maps.t
> b/tests/monitor/testcases/set-maps.t
> index d94016beb0767..6ea36cb9d11d6 100644
> --- a/tests/monitor/testcases/set-maps.t
> +++ b/tests/monitor/testcases/set-maps.t
> @@ -2,7 +2,7 @@
> I add table ip t
> O add table ip t
> I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
> -O add map ip t portip { type inet_service : ipv4_addr;flags interval }
> +O add map ip t portip { type inet_service : ipv4_addr;flags interval; }
So the proposal is to remove the whitespace? I think it's more
readable the way it is already.
Unless I'm overlooking anything, please, leave it as it is. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
