Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[Thomas Monjalon]

01/05/2019 21:50, Bruce Richardson:
> This set of changes fixes warnings seen when compiling DPDK on Fedora 30.
> In most cases these warnings appear to be false positives, which means we
> have the option to just disable the warning. Because the changes required
> to the code to silence the warnings are fairly small I've chosen in all cases
> to change the code rather than disable the warnings, but I'm open to doing
> the opposite if it's felt it's a better solution. [One thing I didn't like
> about disabling the warnings is that the disabling flags are not supported
> by clang, so adding them involves compiler checks :-(]
> 
> NOTE: this set does not cover all warnings with GCC9, but it does cover
> those seen when building with meson. There is still one warning disable
> flag needed when building with make, which will need a follow-on set to
> fix.
> 
> Bruce Richardson (4):
>   net/ixgbe: fix warning with GCC 9 on Fedora 30
>   bus/fslmc: fix printf of null pointer
>   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
>   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30

Cc: stable@dpdk.org

Applied, thanks

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[David Marchand]

On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> 01/05/2019 21:50, Bruce Richardson:
> > This set of changes fixes warnings seen when compiling DPDK on Fedora 30.
> > In most cases these warnings appear to be false positives, which means we
> > have the option to just disable the warning. Because the changes required
> > to the code to silence the warnings are fairly small I've chosen in all
> cases
> > to change the code rather than disable the warnings, but I'm open to
> doing
> > the opposite if it's felt it's a better solution. [One thing I didn't
> like
> > about disabling the warnings is that the disabling flags are not
> supported
> > by clang, so adding them involves compiler checks :-(]
> >
> > NOTE: this set does not cover all warnings with GCC9, but it does cover
> > those seen when building with meson. There is still one warning disable
> > flag needed when building with make, which will need a follow-on set to
> > fix.
> >
> > Bruce Richardson (4):
> >   net/ixgbe: fix warning with GCC 9 on Fedora 30
> >   bus/fslmc: fix printf of null pointer
> >   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
> >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30
>
> Cc: stable@dpdk.org
>
> Applied, thanks
>
>
I had a comment on patch 2, and the bigger problem is
-Waddress-of-packed-member.
The quicker solution for now is to downgrade it to warning only so that we
can fix the parts later rather than globally disable it.


-- 
David Marchand

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[Bruce Richardson]

On Thu, May 02, 2019 at 02:32:41PM +0200, David Marchand wrote:
>    On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon <[1]thomas@monjalon.net>
>    wrote:
> 
>      01/05/2019 21:50, Bruce Richardson:
>      > This set of changes fixes warnings seen when compiling DPDK on
>      Fedora 30.
>      > In most cases these warnings appear to be false positives, which
>      means we
>      > have the option to just disable the warning. Because the changes
>      required
>      > to the code to silence the warnings are fairly small I've chosen
>      in all cases
>      > to change the code rather than disable the warnings, but I'm open
>      to doing
>      > the opposite if it's felt it's a better solution. [One thing I
>      didn't like
>      > about disabling the warnings is that the disabling flags are not
>      supported
>      > by clang, so adding them involves compiler checks :-(]
>      >
>      > NOTE: this set does not cover all warnings with GCC9, but it does
>      cover
>      > those seen when building with meson. There is still one warning
>      disable
>      > flag needed when building with make, which will need a follow-on
>      set to
>      > fix.
>      >
>      > Bruce Richardson (4):
>      >   net/ixgbe: fix warning with GCC 9 on Fedora 30
>      >   bus/fslmc: fix printf of null pointer
>      >   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
>      >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30
>      Cc: [2]stable@dpdk.org
>      Applied, thanks
> 
>    I had a comment on patch 2, and the bigger problem is
>    -Waddress-of-packed-member.
>    The quicker solution for now is to downgrade it to warning only so that
>    we can fix the parts later rather than globally disable it.
>    --
Well, it is already a warning, it's just that with make we build by default
with -Werror when building from git.
Also, that particular warning is already disabled for clang compilation, so
extending that to being disabled for gcc seems fine for a fix for this
release.

/Bruce

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[David Marchand]

On Thu, May 2, 2019 at 3:24 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, May 02, 2019 at 02:32:41PM +0200, David Marchand wrote:
> >    On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon <[1]
> thomas@monjalon.net>
> >    wrote:
> >
> >      01/05/2019 21:50, Bruce Richardson:
> >      > This set of changes fixes warnings seen when compiling DPDK on
> >      Fedora 30.
> >      > In most cases these warnings appear to be false positives, which
> >      means we
> >      > have the option to just disable the warning. Because the changes
> >      required
> >      > to the code to silence the warnings are fairly small I've chosen
> >      in all cases
> >      > to change the code rather than disable the warnings, but I'm open
> >      to doing
> >      > the opposite if it's felt it's a better solution. [One thing I
> >      didn't like
> >      > about disabling the warnings is that the disabling flags are not
> >      supported
> >      > by clang, so adding them involves compiler checks :-(]
> >      >
> >      > NOTE: this set does not cover all warnings with GCC9, but it does
> >      cover
> >      > those seen when building with meson. There is still one warning
> >      disable
> >      > flag needed when building with make, which will need a follow-on
> >      set to
> >      > fix.
> >      >
> >      > Bruce Richardson (4):
> >      >   net/ixgbe: fix warning with GCC 9 on Fedora 30
> >      >   bus/fslmc: fix printf of null pointer
> >      >   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
> >      >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30
> >      Cc: [2]stable@dpdk.org
> >      Applied, thanks
> >
> >    I had a comment on patch 2, and the bigger problem is
> >    -Waddress-of-packed-member.
> >    The quicker solution for now is to downgrade it to warning only so
> that
> >    we can fix the parts later rather than globally disable it.
> >    --
> Well, it is already a warning, it's just that with make we build by default
> with -Werror when building from git.
>

Err, why don't we have -Werror for meson ?


Also, that particular warning is already disabled for clang compilation, so
> extending that to being disabled for gcc seems fine for a fix for this
> release.
>

Arf, indeed..


-- 
David Marchand

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[Bruce Richardson]

On Thu, May 02, 2019 at 03:32:20PM +0200, David Marchand wrote:
>    On Thu, May 2, 2019 at 3:24 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Thu, May 02, 2019 at 02:32:41PM +0200, David Marchand wrote:
>      >    On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon
>      <[1][2]thomas@monjalon.net>
>      >    wrote:
>      >
>      >      01/05/2019 21:50, Bruce Richardson:
>      >      > This set of changes fixes warnings seen when compiling DPDK
>      on
>      >      Fedora 30.
>      >      > In most cases these warnings appear to be false positives,
>      which
>      >      means we
>      >      > have the option to just disable the warning. Because the
>      changes
>      >      required
>      >      > to the code to silence the warnings are fairly small I've
>      chosen
>      >      in all cases
>      >      > to change the code rather than disable the warnings, but
>      I'm open
>      >      to doing
>      >      > the opposite if it's felt it's a better solution. [One
>      thing I
>      >      didn't like
>      >      > about disabling the warnings is that the disabling flags
>      are not
>      >      supported
>      >      > by clang, so adding them involves compiler checks :-(]
>      >      >
>      >      > NOTE: this set does not cover all warnings with GCC9, but
>      it does
>      >      cover
>      >      > those seen when building with meson. There is still one
>      warning
>      >      disable
>      >      > flag needed when building with make, which will need a
>      follow-on
>      >      set to
>      >      > fix.
>      >      >
>      >      > Bruce Richardson (4):
>      >      >   net/ixgbe: fix warning with GCC 9 on Fedora 30
>      >      >   bus/fslmc: fix printf of null pointer
>      >      >   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
>      >      >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30
>      >      Cc: [2][3]stable@dpdk.org
>      >      Applied, thanks
>      >
>      >    I had a comment on patch 2, and the bigger problem is
>      >    -Waddress-of-packed-member.
>      >    The quicker solution for now is to downgrade it to warning only
>      so that
>      >    we can fix the parts later rather than globally disable it.
>      >    --
>      Well, it is already a warning, it's just that with make we build by
>      default
>      with -Werror when building from git.
> 
>    Err, why don't we have -Werror for meson ?
> 

Because it's generally not a good idea to use -Werror by default. However,
the test-meson-build script (which we should all be using for test
compilation before upstreaming) sets it for all builds.

/Bruce

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[David Marchand]

On Thu, May 2, 2019 at 3:46 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, May 02, 2019 at 03:32:20PM +0200, David Marchand wrote:
> >    On Thu, May 2, 2019 at 3:24 PM Bruce Richardson
> >    <[1]bruce.richardson@intel.com> wrote:
> >
> >      On Thu, May 02, 2019 at 02:32:41PM +0200, David Marchand wrote:
> >      >    On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon
> >      <[1][2]thomas@monjalon.net>
> >      >    wrote:
> >      >
> >      >      01/05/2019 21:50, Bruce Richardson:
> >      >      > This set of changes fixes warnings seen when compiling DPDK
> >      on
> >      >      Fedora 30.
> >      >      > In most cases these warnings appear to be false positives,
> >      which
> >      >      means we
> >      >      > have the option to just disable the warning. Because the
> >      changes
> >      >      required
> >      >      > to the code to silence the warnings are fairly small I've
> >      chosen
> >      >      in all cases
> >      >      > to change the code rather than disable the warnings, but
> >      I'm open
> >      >      to doing
> >      >      > the opposite if it's felt it's a better solution. [One
> >      thing I
> >      >      didn't like
> >      >      > about disabling the warnings is that the disabling flags
> >      are not
> >      >      supported
> >      >      > by clang, so adding them involves compiler checks :-(]
> >      >      >
> >      >      > NOTE: this set does not cover all warnings with GCC9, but
> >      it does
> >      >      cover
> >      >      > those seen when building with meson. There is still one
> >      warning
> >      >      disable
> >      >      > flag needed when building with make, which will need a
> >      follow-on
> >      >      set to
> >      >      > fix.
> >      >      >
> >      >      > Bruce Richardson (4):
> >      >      >   net/ixgbe: fix warning with GCC 9 on Fedora 30
> >      >      >   bus/fslmc: fix printf of null pointer
> >      >      >   raw/skeleton_rawdev: fix warnings with GCC 9 on Fedora 30
> >      >      >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora 30
> >      >      Cc: [2][3]stable@dpdk.org
> >      >      Applied, thanks
> >      >
> >      >    I had a comment on patch 2, and the bigger problem is
> >      >    -Waddress-of-packed-member.
> >      >    The quicker solution for now is to downgrade it to warning only
> >      so that
> >      >    we can fix the parts later rather than globally disable it.
> >      >    --
> >      Well, it is already a warning, it's just that with make we build by
> >      default
> >      with -Werror when building from git.
> >
> >    Err, why don't we have -Werror for meson ?
> >
>
> Because it's generally not a good idea to use -Werror by default. However,
> the test-meson-build script (which we should all be using for test
> compilation before upstreaming) sets it for all builds.
>

Yes ok, so that old releases still build on newer toolchains.
As for the test-meson-builds.sh and test-build.sh scripts, they are still
widely unknown except by maintainers.
But at least, the ci build script would catch the errors, since it
configures with "meson build --werror -Dexamples=all $OPTS"

-- 
David Marchand

Re: [dpdk-stable] [dpdk-dev] [PATCH 0/4] fix warnings with gcc 9 on Fedora 30

[Bruce Richardson]

On Thu, May 02, 2019 at 03:53:36PM +0200, David Marchand wrote:
>    On Thu, May 2, 2019 at 3:46 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Thu, May 02, 2019 at 03:32:20PM +0200, David Marchand wrote:
>      >    On Thu, May 2, 2019 at 3:24 PM Bruce Richardson
>      >    <[1][2]bruce.richardson@intel.com> wrote:
>      >
>      >      On Thu, May 02, 2019 at 02:32:41PM +0200, David Marchand
>      wrote:
>      >      >    On Thu, May 2, 2019 at 2:19 PM Thomas Monjalon
>      >      <[1][2][3]thomas@monjalon.net>
>      >      >    wrote:
>      >      >
>      >      >      01/05/2019 21:50, Bruce Richardson:
>      >      >      > This set of changes fixes warnings seen when
>      compiling DPDK
>      >      on
>      >      >      Fedora 30.
>      >      >      > In most cases these warnings appear to be false
>      positives,
>      >      which
>      >      >      means we
>      >      >      > have the option to just disable the warning. Because
>      the
>      >      changes
>      >      >      required
>      >      >      > to the code to silence the warnings are fairly small
>      I've
>      >      chosen
>      >      >      in all cases
>      >      >      > to change the code rather than disable the warnings,
>      but
>      >      I'm open
>      >      >      to doing
>      >      >      > the opposite if it's felt it's a better solution.
>      [One
>      >      thing I
>      >      >      didn't like
>      >      >      > about disabling the warnings is that the disabling
>      flags
>      >      are not
>      >      >      supported
>      >      >      > by clang, so adding them involves compiler checks
>      :-(]
>      >      >      >
>      >      >      > NOTE: this set does not cover all warnings with
>      GCC9, but
>      >      it does
>      >      >      cover
>      >      >      > those seen when building with meson. There is still
>      one
>      >      warning
>      >      >      disable
>      >      >      > flag needed when building with make, which will need
>      a
>      >      follow-on
>      >      >      set to
>      >      >      > fix.
>      >      >      >
>      >      >      > Bruce Richardson (4):
>      >      >      >   net/ixgbe: fix warning with GCC 9 on Fedora 30
>      >      >      >   bus/fslmc: fix printf of null pointer
>      >      >      >   raw/skeleton_rawdev: fix warnings with GCC 9 on
>      Fedora 30
>      >      >      >   raw/dpaa2_cmdif: fix warnings with GCC 9 on Fedora
>      30
>      >      >      Cc: [2][3][4]stable@dpdk.org
>      >      >      Applied, thanks
>      >      >
>      >      >    I had a comment on patch 2, and the bigger problem is
>      >      >    -Waddress-of-packed-member.
>      >      >    The quicker solution for now is to downgrade it to
>      warning only
>      >      so that
>      >      >    we can fix the parts later rather than globally disable
>      it.
>      >      >    --
>      >      Well, it is already a warning, it's just that with make we
>      build by
>      >      default
>      >      with -Werror when building from git.
>      >
>      >    Err, why don't we have -Werror for meson ?
>      >
>      Because it's generally not a good idea to use -Werror by default.
>      However,
>      the test-meson-build script (which we should all be using for test
>      compilation before upstreaming) sets it for all builds.
> 
>    Yes ok, so that old releases still build on newer toolchains.
>    As for the test-meson-builds.sh and test-build.sh scripts, they are
>    still widely unknown except by maintainers.
>    But at least, the ci build script would catch the errors, since it
>    configures with "meson build --werror -Dexamples=all $OPTS"
>    --

Yep, that's the idea. So long as it's easy enough for maintainers and CI to
use werror, we should be covered.