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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux