Re: [PATCH 1/2] xfs: don't take addresses of packed xfs_agfl_t member

Eric Sandeen
2020-01-29T18:18:19Z

On 1/29/20 12:09 PM, Christoph Hellwig wrote:
> On Wed, Jan 29, 2020 at 11:43:13AM -0600, Eric Sandeen wrote:
>> gcc now warns about taking an address of a packed structure member.
>>
>> Work around this by using offsetof() instead.
>>
>> Thanks to bfoster for the suggestion and djwong for reiterating it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
>> index 1b7dcbae051c..7bfc8e2437e9 100644
>> --- a/fs/xfs/libxfs/xfs_format.h
>> +++ b/fs/xfs/libxfs/xfs_format.h
>> @@ -787,7 +787,8 @@ typedef struct xfs_agi {
>>
>> #define XFS_BUF_TO_AGFL_BNO(mp, bp) \
>> (xfs_sb_version_hascrc(&((mp)->m_sb)) ? \
>> - &(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \
>> + (__be32 *)((char *)(bp)->b_addr + \
>> + offsetof(struct xfs_agfl, agfl_bno)) : \
>> (__be32 *)(bp)->b_addr)
>
> Yikes. If we want to go down this route this really needs to become
> an inline function (and fiven that it touches buffer is has no business
> in xfs_format.h).

fair point

>
> But I absolutely do not see the point. If agfl_bno was unalgined
> so is adding the offsetoff. The warnings makes no sense, and there is
> a good reason the kernel build turned it off.

Why do the warnings make no sense?

TBH, the above construction actually makes a lot more intuitive sense to
me, alignment concerns or not.

-Eric