"J. Bruce Fields" <[email protected]>
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 =
=20
>> _continue_ to define these" -- meaning, I'm not changing the structu=
res.
>
> Err, but that's not right either, is it?: We don't need to apply thi=
s
> patch in order to continue to define the structures as they're curren=
tly
> 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.
--=20
M=E5ns Rullg=E5rd
[email protected]