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

["Måns Rullgård" <mans-2StjZFpD7GcAvxtiuMwx3w@public.gmane.org>]

"J. Bruce Fields" <bfields@fieldses.org>
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
mans-2StjZFpD7GcAvxtiuMwx3w@public.gmane.org