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

[Nan KX Li]

Hi Cyril and Jeremy,
I'm sorry for that. 
This morning I changed my code and accidentally pushed to github before I 
found your reviews. I will be more careful in future.

Regards,

William Li ( Li Nan, 李楠 ) 
 

Firmware Engineering Professional
OpenPower AE Team | IBM System & Technology Lab
Mobile: +86-186-1081 6605

Beijing, China



From:   Cyril Bur <cyrilbur at gmail.com>
To:     Nan KX Li/China/IBM at IBMCN
Cc:     Jeremy Kerr <jk at ozlabs.org>, openbmc at lists.ozlabs.org
Date:   05/26/2016 12:04
Subject:        Re: [PATCH openpower-host-ipmi-oem v2] Fix endianness 
issue5



On Thu, 26 May 2016 10:25:34 +0800
Jeremy Kerr <jk at 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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160526/87dec982/attachment.html>