Re: [PATCH openpower-host-ipmi-oem v2] Fix endianness issue5

[Cyril Bur <cyrilbur@gmail.com>]

On Thu, 26 May 2016 10:25:34 +0800
Jeremy Kerr <jk@ozlabs.org> wrote:

> Hi Nan,
> 

Hi Nan,

I responded to v1 of your patch. You have sent exactly the same code as v2. 

One of two things has happened:
One you accidentally triggered github to send a v2 (which admittedly is easy to
do), please try to be more careful in future, this creates a lot of pointless
noise on the list and leads to double reviews, Jeremy has made the same point
I previously have.
Two you're ignoring reviews to your patches and sending subsequent versions...
why do you do this?

> You seem to have a minor typo in the commit title there.
> 
> > -	// TODO: Issue 5: This is not endian-safe.
> > -	short *recid  =  (short*) &reqptr->selrecordls;
> > -	short *offset =  (short*) &reqptr->offsetls;
> > +
> > +	unsigned short recid  =  ((unsigned short) reqptr->selrecordms) << 8 + reqptr->selrecordls;
> > +	unsigned short offset =  ((unsigned short) reqptr->offsetms) << 8 + reqptr->offsetls;  
> 
> That won't do the endian conversion correctly. Can you use one of the
> existing endian conversion functions?
> 
> The le16toh() function is probably what you want here, from endian.h.
> 
> Also, be careful of the change from 'short' to 'unsigned short'.
> 
> Regards,
> 
> 
> Jeremy
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc