Re: [PATCH bluetooth-next 2/3] ieee802154: cleanup ieee802154_be64_to_le64

[Alexander Aring <alex.aring@gmail.com>]

On Wed, Feb 11, 2015 at 02:06:41PM +0100, Alexander Aring wrote:
> On Wed, Feb 11, 2015 at 01:59:58PM +0100, Phoebe Buckheister wrote:
> > On Wed, 11 Feb 2015 13:36:03 +0100
> > Alexander Aring <alex.aring@gmail.com> wrote:
> > 
> > > On Wed, Feb 11, 2015 at 01:17:50PM +0100, Marc Kleine-Budde wrote:
> > > > On 02/10/2015 11:14 PM, Alexander Aring wrote:
> > > > > This patch cleanups the ieee802154_be64_to_le64 function. This
> > > > > patch removes an unnecessary temporary variable.
> > > > > 
> > > > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > > > ---
> > > > >  include/net/mac802154.h | 4 +---
> > > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/include/net/mac802154.h b/include/net/mac802154.h
> > > > > index 8506478..a4dcefe 100644
> > > > > --- a/include/net/mac802154.h
> > > > > +++ b/include/net/mac802154.h
> > > > > @@ -233,9 +233,7 @@ struct ieee802154_ops {
> > > > >   */
> > > > >  static inline void ieee802154_be64_to_le64(void *le64_dst, const
> > > > > void *be64_src) {
> > > > > -	__le64 tmp = (__force __le64)swab64p(be64_src);
> > > > > -
> > > > > -	memcpy(le64_dst, &tmp, IEEE802154_EXTENDED_ADDR_LEN);
> > > > > +	*((__le64 *)le64_dst) = (__force
> > > > > __le64)swab64p(be64_src);
> > > > 
> > > > I assume the compiler optimizes the memcpy, due to the constant
> > > > length argument. It the dst always 64 bit aligned?
> > > 
> > > should be. I can't check this, but dst and src should always 64 bit
> > > long. (Otherwise the programmer do something wrong).
> > > 
> > > It's just byte swapping a little endian 64 bit value to big endian 64
> > > bit value. Because mac header is little and the most netdev core api
> > > uses big endian. These function are some helper function to transfer
> > > __le64 to __be64. Sometimes the __le64 and __be64 are implementated as
> > > arrays so I do this over some void pointers, like [0].
> > 
> > If you convert into an array (which happens sometimes, as you point
> > out), you cannot assume alignment stronger than single bytes. If you do
> > remove the memcpy, please adjust the dst argument pointer to be
> > __le64* to catch these cases of possible misalignment.
> > 
> 
> ah, I see (maybe).
> 
> Then I should use more "put_unaligned_le64" for ieee802154_be64_to_le64
> and "put_unaligned_be64" for ieee802154_le64_to_be64.
> 

ehm, I mean just do put_unaligned64 in both functions. I need to run swab64p
in every endianess case. Doesn't depend if I am on a big or little endian
machine.

- Alex