mbox series

[nft,v2,0/8] fix compiler warnings with clang

Message ID 20230829125809.232318-1-thaller@redhat.com
Headers show
Series fix compiler warnings with clang | expand

Message

Thomas Haller Aug. 29, 2023, 12:53 p.m. UTC
Building with clang caused some compiler warnings. Fix, suppress or work
around them.

Changes to v1:
- replace patches
    "src: use "%zx" format instead of "%Zx""
    "utils: add _NFT_PRAGMA_WARNING_DISABLE()/_NFT_PRAGMA_WARNING_REENABLE helpers"
    "datatype: suppress "-Wformat-nonliteral" warning in integer_type_print()"
  with
    "include: drop "format" attribute from nft_gmp_print()"
  which is the better solution.
- let SNPRINTF_BUFFER_SIZE() not assert against truncation. Instead, the
  callers handle it.
- add bugfix "evaluate: fix check for truncation in stmt_evaluate_log_prefix()"
- add minor patch "evaluate: don't needlessly clear full string buffer in stmt_evaluate_log_prefix()"

Thomas Haller (8):
  netlink: avoid "-Wenum-conversion" warning in dtype_map_from_kernel()
  netlink: avoid "-Wenum-conversion" warning in parser_bison.y
  datatype: avoid cast-align warning with struct sockaddr result from
    getaddrinfo()
  evaluate: fix check for truncation in stmt_evaluate_log_prefix()
  src: rework SNPRINTF_BUFFER_SIZE() and handle truncation
  evaluate: don't needlessly clear full string buffer in
    stmt_evaluate_log_prefix()
  src: suppress "-Wunused-but-set-variable" warning with
    "parser_bison.c"
  include: drop "format" attribute from nft_gmp_print()

 include/nftables.h |  3 +--
 include/utils.h    | 35 ++++++++++++++++++++++++++---------
 src/Makefile.am    |  1 +
 src/datatype.c     | 14 +++++++++++---
 src/evaluate.c     | 15 ++++++++++-----
 src/meta.c         | 11 ++++++-----
 src/netlink.c      |  2 +-
 src/parser_bison.y |  4 ++--
 8 files changed, 58 insertions(+), 27 deletions(-)

Comments

Pablo Neira Ayuso Aug. 29, 2023, 3 p.m. UTC | #1
On Tue, Aug 29, 2023 at 02:53:29PM +0200, Thomas Haller wrote:
> Building with clang caused some compiler warnings. Fix, suppress or work
> around them.

Series LGTM, applied.

The ugly macro can go away later, once meta_parse_key() is removed
when bison/flex use start conditions for this, and probably log prefix
is reviewed not to use it anymore. I still think that macro is not
looking any better after this update but this is not a deal breaker
for this series.

BTW, would you extend tests/build to check for run build tests for
clang in a follow up patch? That would help a lot to improve coverage,
and reduce chances compilation with clang breaks again in the future
before a release.

Thanks.

> Changes to v1:
> - replace patches
>     "src: use "%zx" format instead of "%Zx""
>     "utils: add _NFT_PRAGMA_WARNING_DISABLE()/_NFT_PRAGMA_WARNING_REENABLE helpers"
>     "datatype: suppress "-Wformat-nonliteral" warning in integer_type_print()"
>   with
>     "include: drop "format" attribute from nft_gmp_print()"
>   which is the better solution.
> - let SNPRINTF_BUFFER_SIZE() not assert against truncation. Instead, the
>   callers handle it.
> - add bugfix "evaluate: fix check for truncation in stmt_evaluate_log_prefix()"
> - add minor patch "evaluate: don't needlessly clear full string buffer in stmt_evaluate_log_prefix()"
> 
> Thomas Haller (8):
>   netlink: avoid "-Wenum-conversion" warning in dtype_map_from_kernel()
>   netlink: avoid "-Wenum-conversion" warning in parser_bison.y
>   datatype: avoid cast-align warning with struct sockaddr result from
>     getaddrinfo()
>   evaluate: fix check for truncation in stmt_evaluate_log_prefix()
>   src: rework SNPRINTF_BUFFER_SIZE() and handle truncation
>   evaluate: don't needlessly clear full string buffer in
>     stmt_evaluate_log_prefix()
>   src: suppress "-Wunused-but-set-variable" warning with
>     "parser_bison.c"
>   include: drop "format" attribute from nft_gmp_print()
> 
>  include/nftables.h |  3 +--
>  include/utils.h    | 35 ++++++++++++++++++++++++++---------
>  src/Makefile.am    |  1 +
>  src/datatype.c     | 14 +++++++++++---
>  src/evaluate.c     | 15 ++++++++++-----
>  src/meta.c         | 11 ++++++-----
>  src/netlink.c      |  2 +-
>  src/parser_bison.y |  4 ++--
>  8 files changed, 58 insertions(+), 27 deletions(-)
> 
> -- 
> 2.41.0
>
Thomas Haller Aug. 29, 2023, 5:52 p.m. UTC | #2
On Tue, 2023-08-29 at 17:00 +0200, Pablo Neira Ayuso wrote:
> On Tue, Aug 29, 2023 at 02:53:29PM +0200, Thomas Haller wrote:
> > Building with clang caused some compiler warnings. Fix, suppress or
> > work
> > around them.
> 
> Series LGTM, applied.


Thank you!!

> 
> The ugly macro can go away later, once meta_parse_key() is removed
> when bison/flex use start conditions for this, and probably log
> prefix
> is reviewed not to use it anymore. I still think that macro is not
> looking any better after this update but this is not a deal breaker
> for this series.

I don't find that macro that ugly (anymore). But OK.

> 
> BTW, would you extend tests/build to check for run build tests for
> clang in a follow up patch? That would help a lot to improve
> coverage,
> and reduce chances compilation with clang breaks again in the future
> before a release.

Yes, I have several more patches, that I will send soon.


Thomas

> 
> Thanks.
> 
> > Changes to v1:
> > - replace patches
> >     "src: use "%zx" format instead of "%Zx""
> >     "utils: add
> > _NFT_PRAGMA_WARNING_DISABLE()/_NFT_PRAGMA_WARNING_REENABLE helpers"
> >     "datatype: suppress "-Wformat-nonliteral" warning in
> > integer_type_print()"
> >   with
> >     "include: drop "format" attribute from nft_gmp_print()"
> >   which is the better solution.
> > - let SNPRINTF_BUFFER_SIZE() not assert against truncation.
> > Instead, the
> >   callers handle it.
> > - add bugfix "evaluate: fix check for truncation in
> > stmt_evaluate_log_prefix()"
> > - add minor patch "evaluate: don't needlessly clear full string
> > buffer in stmt_evaluate_log_prefix()"
> > 
> > Thomas Haller (8):
> >   netlink: avoid "-Wenum-conversion" warning in
> > dtype_map_from_kernel()
> >   netlink: avoid "-Wenum-conversion" warning in parser_bison.y
> >   datatype: avoid cast-align warning with struct sockaddr result
> > from
> >     getaddrinfo()
> >   evaluate: fix check for truncation in stmt_evaluate_log_prefix()
> >   src: rework SNPRINTF_BUFFER_SIZE() and handle truncation
> >   evaluate: don't needlessly clear full string buffer in
> >     stmt_evaluate_log_prefix()
> >   src: suppress "-Wunused-but-set-variable" warning with
> >     "parser_bison.c"
> >   include: drop "format" attribute from nft_gmp_print()
> > 
> >  include/nftables.h |  3 +--
> >  include/utils.h    | 35 ++++++++++++++++++++++++++---------
> >  src/Makefile.am    |  1 +
> >  src/datatype.c     | 14 +++++++++++---
> >  src/evaluate.c     | 15 ++++++++++-----
> >  src/meta.c         | 11 ++++++-----
> >  src/netlink.c      |  2 +-
> >  src/parser_bison.y |  4 ++--
> >  8 files changed, 58 insertions(+), 27 deletions(-)
> > 
> > -- 
> > 2.41.0
> > 
>