| Message ID | 20191016230322.24432-5-phil@nwl.cc |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | A bunch of fixes for --echo option | expand |
Phil Sutter <phil@nwl.cc> wrote: > Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > removed an extra semicolon at end of line, but thereby broke single line > output. The correct fix is to use opts->stmt_separator which holds > either newline or semicolon chars depending on output mode. Acked-by: Florian Westphal <fw@strlen.de>
On Thu, Oct 17, 2019 at 01:03:22AM +0200, Phil Sutter wrote: > Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > removed an extra semicolon at end of line, but thereby broke single line > output. The correct fix is to use opts->stmt_separator which holds > either newline or semicolon chars depending on output mode. What output mode this breaks? It looks indeed like I overlook something while fixing up this. Thanks. > Fixes: 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > src/rule.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/rule.c b/src/rule.c > index 2d35bae44c9e5..3c7c8d63f8cdf 100644 > --- a/src/rule.c > +++ b/src/rule.c > @@ -1869,7 +1869,7 @@ static void obj_print_data(const struct obj *obj, > nft_print(octx, "%s", opts->nl); > nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab); > print_proto_name_proto(obj->ct_timeout.l4proto, octx); > - nft_print(octx, "%s", opts->nl); > + nft_print(octx, "%s", opts->stmt_separator); > nft_print(octx, "%s%sl3proto %s%s", > opts->tab, opts->tab, > family2str(obj->ct_timeout.l3proto), > -- > 2.23.0 >
On Thu, Oct 17, 2019 at 01:14:37PM +0200, Pablo Neira Ayuso wrote: > On Thu, Oct 17, 2019 at 01:03:22AM +0200, Phil Sutter wrote: > > Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > > removed an extra semicolon at end of line, but thereby broke single line > > output. The correct fix is to use opts->stmt_separator which holds > > either newline or semicolon chars depending on output mode. > > What output mode this breaks? It looks indeed like I overlook > something while fixing up this. It breaks syntax of monitor and echo output. We don't propagate it, but the goal always has been for those to print syntactically correct commands. The concrete test case in tests/monitor/testcases/object.t is: | add ct timeout ip t ctt { protocol udp; l3proto ip; policy = { unreplied : 15, replied : 12 }; } Omitting the semicolon before 'l3proto' is illegal syntax. Cheers, Phil
On Thu, Oct 17, 2019 at 01:29:10PM +0200, Phil Sutter wrote: > On Thu, Oct 17, 2019 at 01:14:37PM +0200, Pablo Neira Ayuso wrote: > > On Thu, Oct 17, 2019 at 01:03:22AM +0200, Phil Sutter wrote: > > > Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > > > removed an extra semicolon at end of line, but thereby broke single line > > > output. The correct fix is to use opts->stmt_separator which holds > > > either newline or semicolon chars depending on output mode. > > > > What output mode this breaks? It looks indeed like I overlook > > something while fixing up this. > > It breaks syntax of monitor and echo output. We don't propagate it, but > the goal always has been for those to print syntactically correct > commands. > > The concrete test case in tests/monitor/testcases/object.t is: > > | add ct timeout ip t ctt { protocol udp; l3proto ip; policy = { unreplied : 15, replied : 12 }; } > > Omitting the semicolon before 'l3proto' is illegal syntax. Ah, the echo mode indeed. LGTM. Thanks for explaining.
On Thu, Oct 17, 2019 at 01:03:22AM +0200, Phil Sutter wrote: > Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > removed an extra semicolon at end of line, but thereby broke single line > output. The correct fix is to use opts->stmt_separator which holds > either newline or semicolon chars depending on output mode. > > Fixes: 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") > Signed-off-by: Phil Sutter <phil@nwl.cc> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > src/rule.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/rule.c b/src/rule.c > index 2d35bae44c9e5..3c7c8d63f8cdf 100644 > --- a/src/rule.c > +++ b/src/rule.c > @@ -1869,7 +1869,7 @@ static void obj_print_data(const struct obj *obj, > nft_print(octx, "%s", opts->nl); > nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab); > print_proto_name_proto(obj->ct_timeout.l4proto, octx); > - nft_print(octx, "%s", opts->nl); > + nft_print(octx, "%s", opts->stmt_separator); > nft_print(octx, "%s%sl3proto %s%s", > opts->tab, opts->tab, > family2str(obj->ct_timeout.l3proto), > -- > 2.23.0 >
diff --git a/src/rule.c b/src/rule.c index 2d35bae44c9e5..3c7c8d63f8cdf 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1869,7 +1869,7 @@ static void obj_print_data(const struct obj *obj, nft_print(octx, "%s", opts->nl); nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab); print_proto_name_proto(obj->ct_timeout.l4proto, octx); - nft_print(octx, "%s", opts->nl); + nft_print(octx, "%s", opts->stmt_separator); nft_print(octx, "%s%sl3proto %s%s", opts->tab, opts->tab, family2str(obj->ct_timeout.l3proto),
Commit 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") removed an extra semicolon at end of line, but thereby broke single line output. The correct fix is to use opts->stmt_separator which holds either newline or semicolon chars depending on output mode. Fixes: 43ae7a48ae3de ("rule: do not print semicolon in ct timeout") Signed-off-by: Phil Sutter <phil@nwl.cc> --- src/rule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)