Re: [PATCH 19/19] lockd: clean up 64-bit alignment fix in nsm_init_private()

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

 



"J. Bruce Fields" <bfields@xxxxxxxxxxxx>
writes:

> On Tue, Apr 28, 2009 at 01:24:24PM -0400, Chuck Lever wrote:
>> On Apr 28, 2009, at 12:40 PM, J. Bruce Fields wrote:
>>> On Tue, Apr 28, 2009 at 12:35:50PM -0400, Chuck Lever wrote:
>>>> On Apr 28, 2009, at 12:31 PM, J. Bruce Fields wrote:
>>>>> On Thu, Apr 23, 2009 at 07:33:40PM -0400, Chuck Lever wrote:
>>>>>> Recently, commit ad5b365c fixed a data type alignment issue in
>>>>>> nsm_init_private() by introducing put_unaligned().  We don't
>>>>>> actually _require_ an unaligned access to nsm_private here.
>>>>>>
>>>>>> Instead, we should always use memcpy/memcmp to access the
>>>>>> contents of RPC opaque data types.  This permits us to continue
>>>>>> to define these as simple character arrays, as most legacy RPC
>>>>>> code does, and to rely on gcc to pack the fields in in-core
>>>>>> structures optimally without breaking on platforms that require
>>>>>> strict alignment.
>>>>>
>>>>> OK, I'm confused.  What structures will get packed differently?
>>>>
>>>> Any struct that has an nsm_private embedded in it, such as struct
>>>> nlm_reboot.
>>>
>>> I don't see how that or any structure is changed by this patch.
>>
>> It's not.  Note the phrase above in the description: "permits us to  
>> _continue_ to define these" -- meaning, I'm not changing the structures.
>
> Err, but that's not right either, is it?:  We don't need to apply this
> patch in order to continue to define the structures as they're currently
> defined.
>
> Help! I'm confused!

I too fail to see the reasoning with the memcpy().  It achieves
exactly the same end result using twice as much work.  Also, what if
SM_PRIV_SIZE is ever increased?  Then it will be copying "random" data
from the stack into the nsm_private data.  This strikes me as a little
fragile.

-- 
Måns Rullgård
mans@xxxxxxxxx

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux