xfsprogs: don't warn about packed members
diff mbox series

Message ID 20191216215245.13666-1-david@fromorbit.com
State Accepted
Headers show
Series
  • xfsprogs: don't warn about packed members
Related show

Commit Message

Dave Chinner Dec. 16, 2019, 9:52 p.m. UTC
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>
---
 include/builddefs.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Brian Foster Dec. 17, 2019, 11:54 a.m. UTC | #1
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
>
Darrick J. Wong Dec. 17, 2019, 5:09 p.m. UTC | #2
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
> > 
>
Christoph Hellwig Jan. 26, 2020, 11:02 a.m. UTC | #3
Eric, can you pick this one up?  The warnings are fairly annoying..
Eric Sandeen Jan. 27, 2020, 5:43 p.m. UTC | #4
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
Christoph Hellwig Jan. 29, 2020, 6:46 a.m. UTC | #5
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.
Christoph Hellwig March 12, 2020, 2:09 p.m. UTC | #6
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..
Eric Sandeen March 13, 2020, 2:06 p.m. UTC | #7
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
Eric Sandeen March 13, 2020, 2:07 p.m. UTC | #8
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>

Patch
diff mbox series

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@