[nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[Phil Sutter]

Eliminate as many bison warnings emitted since bison-3.3 as possible.
Sadly getting bison, flex and automake right is full of pitfalls so on
one hand this series does not fix for deprecated %name-prefix statement
and on the other passes -Wno-yacc to bison to not complain about POSIX
incompatibilities although automake causes to run bison in POSIX compat
mode in the first place. Fixing either of those turned out to be
non-trivial.

Changes since v1:
- Drop nfnl_osf patch, Fernando took care of that already.
- Split remaining patch in two.
- Document which warnings are being silenced.

Phil Sutter (2):
  parser_bison: Fix for deprecated statements
  src: Call bison with -Wno-yacc to silence warnings

 src/Makefile.am    | 2 +-
 src/parser_bison.y | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.22.0

[nft PATCH v2 1/2] parser_bison: Fix for deprecated statements

[Phil Sutter]

Bison-3.3 started to warn about:

/home/n0-1/git/nftables/src/parser_bison.y:117.1-19: warning: deprecated directive, use ‘%define api.prefix {nft_}’ [-Wdeprecated]
    117 | %name-prefix "nft_"
        | ^~~~~~~~~~~~~~~~~~~
/home/n0-1/git/nftables/src/parser_bison.y:119.1-12: warning: deprecated directive, use ‘%define api.pure’ [-Wdeprecated]
  119 | %pure-parser
      | ^~~~~~~~~~~~
/home/n0-1/git/nftables/src/parser_bison.y:124.1-14: warning: deprecated directive, use ‘%define parse.error verbose’ [-Wdeprecated]
  124 | %error-verbose
      | ^~~~~~~~~~~~~~

Replace the last two as suggested but leave the first one in place as
that causes compilation errors in scanner.l - flex seems not to pick up
the changed internal symbol names.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/parser_bison.y | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 53e669521efa0..b463a140d31ff 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -116,12 +116,12 @@ int nft_lex(void *, void *, void *);
 
 %name-prefix "nft_"
 %debug
-%pure-parser
+%define api.pure
 %parse-param		{ struct nft_ctx *nft }
 %parse-param		{ void *scanner }
 %parse-param		{ struct parser_state *state }
 %lex-param		{ scanner }
-%error-verbose
+%define parse.error verbose
 %locations
 
 %initial-action {
-- 
2.22.0

[nft PATCH v2 2/2] src: Call bison with -Wno-yacc to silence warnings

[Phil Sutter]

Bison-3.3 significantly increased warnings for POSIX incompatibilities,
it now complains about missing support for %name-prefix, %define,
%destructor and string literals. The latter applies to parameter of
%name-prefix and all relevant %token statements.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index e2b531390cefb..740c21f2cac85 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -22,7 +22,7 @@ AM_CFLAGS = -Wall								\
 	    -Waggregate-return -Wunused -Wwrite-strings ${GCC_FVISIBILITY_HIDDEN}
 
 
-AM_YFLAGS = -d
+AM_YFLAGS = -d -Wno-yacc
 
 BUILT_SOURCES = parser_bison.h
 
-- 
2.22.0

Re: [nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[Pablo Neira Ayuso]

On Tue, Jul 23, 2019 at 03:23:11PM +0200, Phil Sutter wrote:
> Eliminate as many bison warnings emitted since bison-3.3 as possible.
> Sadly getting bison, flex and automake right is full of pitfalls so on
> one hand this series does not fix for deprecated %name-prefix statement
> and on the other passes -Wno-yacc to bison to not complain about POSIX
> incompatibilities although automake causes to run bison in POSIX compat
> mode in the first place. Fixing either of those turned out to be
> non-trivial.

Indeed, lots of warnings and things to be updated.

Do you think it's worth fixing those in the midterm?

We can just place these two small ones in the tree, I'm just concerned
about tech debt in the midterm, these deprecated stuff might just go
away.

Thanks.

> Changes since v1:
> - Drop nfnl_osf patch, Fernando took care of that already.
> - Split remaining patch in two.
> - Document which warnings are being silenced.
> 
> Phil Sutter (2):
>   parser_bison: Fix for deprecated statements
>   src: Call bison with -Wno-yacc to silence warnings
> 
>  src/Makefile.am    | 2 +-
>  src/parser_bison.y | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> -- 
> 2.22.0
> 

Re: [nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[Phil Sutter]

Hi Pablo,

On Tue, Jul 30, 2019 at 02:41:06PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 23, 2019 at 03:23:11PM +0200, Phil Sutter wrote:
> > Eliminate as many bison warnings emitted since bison-3.3 as possible.
> > Sadly getting bison, flex and automake right is full of pitfalls so on
> > one hand this series does not fix for deprecated %name-prefix statement
> > and on the other passes -Wno-yacc to bison to not complain about POSIX
> > incompatibilities although automake causes to run bison in POSIX compat
> > mode in the first place. Fixing either of those turned out to be
> > non-trivial.
> 
> Indeed, lots of warnings and things to be updated.
> 
> Do you think it's worth fixing those in the midterm?
> 
> We can just place these two small ones in the tree, I'm just concerned
> about tech debt in the midterm, these deprecated stuff might just go
> away.

We should avoid calling bison with -y since the parser simply isn't
POSIX yacc compatible. I found a trick somewhere in WWW to do that (one
has to substitute AC_PROG_YACC) but lost the reference again. But after
doing so, there was a problem with file names I failed to resolve.
Hence why I resorted to just passing -Wno-yacc.

Cheers, Phil

Re: [nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[Pablo Neira Ayuso]

On Tue, Jul 30, 2019 at 04:03:54PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Tue, Jul 30, 2019 at 02:41:06PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Jul 23, 2019 at 03:23:11PM +0200, Phil Sutter wrote:
> > > Eliminate as many bison warnings emitted since bison-3.3 as possible.
> > > Sadly getting bison, flex and automake right is full of pitfalls so on
> > > one hand this series does not fix for deprecated %name-prefix statement
> > > and on the other passes -Wno-yacc to bison to not complain about POSIX
> > > incompatibilities although automake causes to run bison in POSIX compat
> > > mode in the first place. Fixing either of those turned out to be
> > > non-trivial.
> > 
> > Indeed, lots of warnings and things to be updated.
> > 
> > Do you think it's worth fixing those in the midterm?
> > 
> > We can just place these two small ones in the tree, I'm just concerned
> > about tech debt in the midterm, these deprecated stuff might just go
> > away.
> 
> We should avoid calling bison with -y since the parser simply isn't
> POSIX yacc compatible. I found a trick somewhere in WWW to do that (one
> has to substitute AC_PROG_YACC) but lost the reference again. But after
> doing so, there was a problem with file names I failed to resolve.
> Hence why I resorted to just passing -Wno-yacc.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

If parser does not break with this and tests pass fine, I'm ok with
this temporary solution. I also need all most warnings go away here by
now.

Please, if you find that reference again share it with me.

Thanks.

Re: [nft PATCH v2 0/2] parser_bison: Get rid of (most) bison compiler warnings

[Phil Sutter]

Hi,

On Tue, Jul 30, 2019 at 04:18:45PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 30, 2019 at 04:03:54PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > On Tue, Jul 30, 2019 at 02:41:06PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Jul 23, 2019 at 03:23:11PM +0200, Phil Sutter wrote:
> > > > Eliminate as many bison warnings emitted since bison-3.3 as possible.
> > > > Sadly getting bison, flex and automake right is full of pitfalls so on
> > > > one hand this series does not fix for deprecated %name-prefix statement
> > > > and on the other passes -Wno-yacc to bison to not complain about POSIX
> > > > incompatibilities although automake causes to run bison in POSIX compat
> > > > mode in the first place. Fixing either of those turned out to be
> > > > non-trivial.
> > > 
> > > Indeed, lots of warnings and things to be updated.
> > > 
> > > Do you think it's worth fixing those in the midterm?
> > > 
> > > We can just place these two small ones in the tree, I'm just concerned
> > > about tech debt in the midterm, these deprecated stuff might just go
> > > away.
> > 
> > We should avoid calling bison with -y since the parser simply isn't
> > POSIX yacc compatible. I found a trick somewhere in WWW to do that (one
> > has to substitute AC_PROG_YACC) but lost the reference again. But after
> > doing so, there was a problem with file names I failed to resolve.
> > Hence why I resorted to just passing -Wno-yacc.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> If parser does not break with this and tests pass fine, I'm ok with
> this temporary solution. I also need all most warnings go away here by
> now.
> 
> Please, if you find that reference again share it with me.

Sorry, I didn't. But instead I found Bison 3.0 release notes[1], please
have a look at "Use of YACC='bison -y'" paragraph. I tried to set
YACC='bison -o y.tab.c' in configure.ac. This avoids the warnings in
parser_bison.y, but scanner.l fails to build as a macro ECHO is being
defined by bison/flex which clashes with ECHO keyword used by us.

Given that even Bison maintainer suggests to set -Wno-yacc, let's just
wait what autoconf-2.7 brings.

Speaking of parser_bison, we had a discussion at NFWS about whether to
quote all user-defined strings on output (and make sure to accept quotes
on input). Your bet was that it is possible to fix the problem of users
choosing keywords (e.g. "hour" as table name) from within bison. I tried
and came up with the following:

| --- a/src/parser_bison.y
| +++ b/src/parser_bison.y
| @@ -2021,6 +2021,8
|                         ;
|  
|  identifier             :       STRING
| +                       |       HOUR            { $$ = strdup("hour"); }
| +                       |       TABLE           { $$ = strdup("table"); }
|                         ;
|  
|  string                 :       STRING

This works in that I can create a table named "table":

| # nft add table table
| # nft list ruleset
| table ip table {
| }

But it causes shift/reduce warnings when compiling. Do you have an idea
for how to avoid the shift/reduce conflicts?

Cheers, Phil

[1] https://savannah.gnu.org/forum/forum.php?forum_id=7663