| Message ID | 20191216215245.13666-1-david@fromorbit.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfsprogs: don't warn about packed members | expand |
On Tue, Dec 17, 2019 at 08:52:45AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > gcc 9.2.1 throws lots of new warnings during the build like this: > > xfs_format.h:790:3: warning: taking address of packed member of ‘struct xfs_agfl’ may result in an unaligned pointer value [-Waddress-of-packed-member] > 790 | &(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > xfs_alloc.c:3149:13: note: in expansion of macro ‘XFS_BUF_TO_AGFL_BNO’ > 3149 | agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp); > | ^~~~~~~~~~~~~~~~~~~ > > We know this packed structure aligned correctly, so turn off this > warning to shut gcc up. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- I'm wondering if we could just use offsetof() in this case so we don't have to disable a warning for the entire project, particularly if this is triggered by a small number of macros.. Brian > include/builddefs.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/builddefs.in b/include/builddefs.in > index 4700b52706a7..6fdc9ebb70c7 100644 > --- a/include/builddefs.in > +++ b/include/builddefs.in > @@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@ > MALLOCLIB = @malloc_lib@ > LOADERFLAGS = @LDFLAGS@ > LTLDFLAGS = @LDFLAGS@ > -CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 > +CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -Wno-address-of-packed-member > BUILD_CFLAGS = @BUILD_CFLAGS@ -D_FILE_OFFSET_BITS=64 > > LIBRT = @librt@ > -- > 2.24.0.rc0 >
On Tue, Dec 17, 2019 at 06:54:01AM -0500, Brian Foster wrote: > On Tue, Dec 17, 2019 at 08:52:45AM +1100, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > gcc 9.2.1 throws lots of new warnings during the build like this: > > > > xfs_format.h:790:3: warning: taking address of packed member of ‘struct xfs_agfl’ may result in an unaligned pointer value [-Waddress-of-packed-member] > > 790 | &(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \ > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > xfs_alloc.c:3149:13: note: in expansion of macro ‘XFS_BUF_TO_AGFL_BNO’ > > 3149 | agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp); > > | ^~~~~~~~~~~~~~~~~~~ > > > > We know this packed structure aligned correctly, so turn off this > > warning to shut gcc up. > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > --- > > I'm wondering if we could just use offsetof() in this case so we don't > have to disable a warning for the entire project, particularly if this > is triggered by a small number of macros.. ...and maybe kill the shouty macro while we're at it. :) --D > Brian > > > include/builddefs.in | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/builddefs.in b/include/builddefs.in > > index 4700b52706a7..6fdc9ebb70c7 100644 > > --- a/include/builddefs.in > > +++ b/include/builddefs.in > > @@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@ > > MALLOCLIB = @malloc_lib@ > > LOADERFLAGS = @LDFLAGS@ > > LTLDFLAGS = @LDFLAGS@ > > -CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 > > +CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -Wno-address-of-packed-member > > BUILD_CFLAGS = @BUILD_CFLAGS@ -D_FILE_OFFSET_BITS=64 > > > > LIBRT = @librt@ > > -- > > 2.24.0.rc0 > > >
Eric, can you pick this one up? The warnings are fairly annoying..
On 1/26/20 5:02 AM, Christoph Hellwig wrote: > Eric, can you pick this one up? The warnings are fairly annoying.. > Sorry, I had missed this one and/or the feedback on the original patch wasn't resolved. I tend to agree that turning off the warning globally because we know /this/ one is OK seems somewhat suboptimal. Let me take a look again. -Eric
On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: > On 1/26/20 5:02 AM, Christoph Hellwig wrote: > > Eric, can you pick this one up? The warnings are fairly annoying.. > > > > Sorry, I had missed this one and/or the feedback on the original patch > wasn't resolved. I tend to agree that turning off the warning globally > because we know /this/ one is OK seems somewhat suboptimal. > > Let me take a look again. Well, the kernel certainly runs with this warning disabled, mostly because it really isn't a very useful warning to start with.
On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: > On 1/26/20 5:02 AM, Christoph Hellwig wrote: > > Eric, can you pick this one up? The warnings are fairly annoying.. > > > > Sorry, I had missed this one and/or the feedback on the original patch > wasn't resolved. I tend to agree that turning off the warning globally > because we know /this/ one is OK seems somewhat suboptimal. > > Let me take a look again. Can we get this queued up in xfsprogs? These warnings are pretty annoying..
On 3/12/20 9:09 AM, Christoph Hellwig wrote: > On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: >> On 1/26/20 5:02 AM, Christoph Hellwig wrote: >>> Eric, can you pick this one up? The warnings are fairly annoying.. >>> >> >> Sorry, I had missed this one and/or the feedback on the original patch >> wasn't resolved. I tend to agree that turning off the warning globally >> because we know /this/ one is OK seems somewhat suboptimal. >> >> Let me take a look again. > > Can we get this queued up in xfsprogs? These warnings are pretty > annoying.. Ok. I don't really like disabling it globally but if it's good enough for the kernel... I'll add this to 5.5.0 and push out the release. -Eric
On 12/16/19 3:52 PM, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > gcc 9.2.1 throws lots of new warnings during the build like this: > > xfs_format.h:790:3: warning: taking address of packed member of ‘struct xfs_agfl’ may result in an unaligned pointer value [-Waddress-of-packed-member] > 790 | &(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > xfs_alloc.c:3149:13: note: in expansion of macro ‘XFS_BUF_TO_AGFL_BNO’ > 3149 | agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp); > | ^~~~~~~~~~~~~~~~~~~ > > We know this packed structure aligned correctly, so turn off this > warning to shut gcc up. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> I guess if it's good enough for the kernel ... I'll probably add a short note to the commit, and Reviewed-by: Eric Sandeen <sandeen@redhat.com>
On Fri, Mar 13, 2020 at 09:06:38AM -0500, Eric Sandeen wrote: > On 3/12/20 9:09 AM, Christoph Hellwig wrote: > > On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: > >> On 1/26/20 5:02 AM, Christoph Hellwig wrote: > >>> Eric, can you pick this one up? The warnings are fairly annoying.. > >>> > >> > >> Sorry, I had missed this one and/or the feedback on the original patch > >> wasn't resolved. I tend to agree that turning off the warning globally > >> because we know /this/ one is OK seems somewhat suboptimal. > >> > >> Let me take a look again. > > > > Can we get this queued up in xfsprogs? These warnings are pretty > > annoying.. > > > Ok. I don't really like disabling it globally but if it's good enough > for the kernel... I'll add this to 5.5.0 and push out the release. Seems like this still isn't in xfsprogs for-next.
On Fri, Apr 24, 2020 at 03:33:23AM -0700, Christoph Hellwig wrote: > On Fri, Mar 13, 2020 at 09:06:38AM -0500, Eric Sandeen wrote: > > On 3/12/20 9:09 AM, Christoph Hellwig wrote: > > > On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: > > >> On 1/26/20 5:02 AM, Christoph Hellwig wrote: > > >>> Eric, can you pick this one up? The warnings are fairly annoying.. > > >>> > > >> > > >> Sorry, I had missed this one and/or the feedback on the original patch > > >> wasn't resolved. I tend to agree that turning off the warning globally > > >> because we know /this/ one is OK seems somewhat suboptimal. > > >> > > >> Let me take a look again. > > > > > > Can we get this queued up in xfsprogs? These warnings are pretty > > > annoying.. > > > > > > Ok. I don't really like disabling it globally but if it's good enough > > for the kernel... I'll add this to 5.5.0 and push out the release. > > Seems like this still isn't in xfsprogs for-next. Odd, it's in origin/master in my tree as the last commit before v5.5.0: 845e5ef706cb7e29259d6541f43a7029e7dc7898. --D
On 4/24/20 12:42 PM, Darrick J. Wong wrote: > On Fri, Apr 24, 2020 at 03:33:23AM -0700, Christoph Hellwig wrote: >> On Fri, Mar 13, 2020 at 09:06:38AM -0500, Eric Sandeen wrote: >>> On 3/12/20 9:09 AM, Christoph Hellwig wrote: >>>> On Mon, Jan 27, 2020 at 11:43:02AM -0600, Eric Sandeen wrote: >>>>> On 1/26/20 5:02 AM, Christoph Hellwig wrote: >>>>>> Eric, can you pick this one up? The warnings are fairly annoying.. >>>>>> >>>>> >>>>> Sorry, I had missed this one and/or the feedback on the original patch >>>>> wasn't resolved. I tend to agree that turning off the warning globally >>>>> because we know /this/ one is OK seems somewhat suboptimal. >>>>> >>>>> Let me take a look again. >>>> >>>> Can we get this queued up in xfsprogs? These warnings are pretty >>>> annoying.. >>> >>> >>> Ok. I don't really like disabling it globally but if it's good enough >>> for the kernel... I'll add this to 5.5.0 and push out the release. >> >> Seems like this still isn't in xfsprogs for-next. > > Odd, it's in origin/master in my tree as the last commit before v5.5.0: > 845e5ef706cb7e29259d6541f43a7029e7dc7898. seems like it's in for-next, no? https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/tree/include/builddefs.in?h=for-next#n16 -Eric
On Fri, Apr 24, 2020 at 12:52:45PM -0500, Eric Sandeen wrote: > seems like it's in for-next, no? > > https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/tree/include/builddefs.in?h=for-next#n16 Indeed it is. I still saw the warnings, though, but a "make realclean" fixed that. Oh the joys of build systems.. Sorry for the noise.
diff --git a/include/builddefs.in b/include/builddefs.in index 4700b52706a7..6fdc9ebb70c7 100644 --- a/include/builddefs.in +++ b/include/builddefs.in @@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@ MALLOCLIB = @malloc_lib@ LOADERFLAGS = @LDFLAGS@ LTLDFLAGS = @LDFLAGS@ -CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 +CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -Wno-address-of-packed-member BUILD_CFLAGS = @BUILD_CFLAGS@ -D_FILE_OFFSET_BITS=64 LIBRT = @librt@