[PATCH] xfsprogs: don't warn about packed members

[PATCH] xfsprogs: don't warn about packed members

[Dave Chinner]

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(-)

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

Re: [PATCH] xfsprogs: don't warn about packed members

[Brian Foster]

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
> 

Re: [PATCH] xfsprogs: don't warn about packed members

[Darrick J. Wong]

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
> > 
> 

Re: [PATCH] xfsprogs: don't warn about packed members

[Christoph Hellwig]

Eric, can you pick this one up?  The warnings are fairly annoying..

Re: [PATCH] xfsprogs: don't warn about packed members

[Eric Sandeen]

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

Re: [PATCH] xfsprogs: don't warn about packed members

[Christoph Hellwig]

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.

Re: [PATCH] xfsprogs: don't warn about packed members

[Christoph Hellwig]

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..

Re: [PATCH] xfsprogs: don't warn about packed members

[Eric Sandeen]

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

Re: [PATCH] xfsprogs: don't warn about packed members

[Christoph Hellwig]

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.

Re: [PATCH] xfsprogs: don't warn about packed members

[Darrick J. Wong]

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

Re: [PATCH] xfsprogs: don't warn about packed members

[Eric Sandeen]

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

Re: [PATCH] xfsprogs: don't warn about packed members

[Christoph Hellwig]

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.

Re: [PATCH] xfsprogs: don't warn about packed members

[Eric Sandeen]

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>