<font size=2 face="sans-serif">Hi Jeremy,</font><br><br><font size=2 face="sans-serif">I've updated my pull request : </font><a href="https://github.com/openbmc/openpower-host-ipmi-oem/pull/13"><font size=2 color=blue face="sans-serif">https://github.com/openbmc/openpower-host-ipmi-oem/pull/13</font></a><font size=2 face="sans-serif">.</font><br><font size=2 face="sans-serif">Please review it.</font><br><br><font size=2 face="sans-serif">For the alignment requirement, I'd use
unsigned char type to access the requestor's buffer. So:</font><br><br><font size=3>uint8_t *reqptr = (uint8_t *) request; </font><br><br><font size=3>esel_req.resid = le16toh((((uint16_t) reqptr[1])
<< 8) + reqptr[0]); </font><br><font size=3>esel_req.selrecord = le16toh((((uint16_t) reqptr[3]) <<
8) + reqptr[2]); </font><br><font size=3>esel_req.offset = le16toh((((uint16_t) reqptr[5])
<< 8) + reqptr[4]); </font><br><br><br><br><font size=3 color=#424282 face="Calibri">Regards,</font><br><br><font size=3 color=#424282 face="Calibri">William Li ( Li Nan, Àîéª
) </font><br><font size=3 color=#424282 face="Calibri"> </font><br><br><font size=3 color=#424282 face="Calibri">Firmware Engineering Professional</font><br><font size=3 color=#424282 face="Calibri">OpenPower AE Team | IBM System
& Technology Lab</font><br><font size=3 color=#424282 face="Calibri">Mobile: +86-186-1081 6605</font><br><br><font size=3 color=#424282 face="Calibri">Beijing, China</font><br><br><br><br><font size=1 color=#5f5f5f face="sans-serif">From:
</font><font size=1 face="sans-serif">Jeremy Kerr <jk@ozlabs.org></font><br><font size=1 color=#5f5f5f face="sans-serif">To:
</font><font size=1 face="sans-serif">OpenBMC Patches <openbmc-patches@stwcx.xyz>,
openbmc@lists.ozlabs.org</font><br><font size=1 color=#5f5f5f face="sans-serif">Cc:
</font><font size=1 face="sans-serif">Nan KX Li/China/IBM@IBMCN</font><br><font size=1 color=#5f5f5f face="sans-serif">Date:
</font><font size=1 face="sans-serif">05/30/2016 14:08</font><br><font size=1 color=#5f5f5f face="sans-serif">Subject:
</font><font size=1 face="sans-serif">Re: [PATCH openpower-host-ipmi-oem
v3] Fix issue5 for endian safe</font><br><hr noshade><br><br><br><tt><font size=2>Hi Nan,<br><br>> @@ -29,9 +30,8 @@ ipmi_ret_t ipmi_ibm_oem_partial_esel(ipmi_netfn_t
netfn, ipmi_cmd_t cmd,<br>>
esel_request_t *reqptr = (esel_request_t*) request;<br>>
FILE *fp;<br>>
int r = 0;<br>> -
// TODO: Issue 5: This is not endian-safe.<br>> -
short *recid = (short*) &reqptr->selrecordls;<br>> -
short *offset = (short*) &reqptr->offsetls;<br>> +
short recid = le16toh((((unsigned short) reqptr->selrecordms)
<< 8) + reqptr->selrecordls);<br>> +
short offset = le16toh((((unsigned short) reqptr->offsetms) <<
8) + reqptr->offsetls);<br><br>I think that this is less correct than the first attempt, and that's<br>probably due to an incorrect suggestion from me. Sorry for that.<br><br>I've only just noticed that you're using different members of the reqptr<br>struct to construct that 16-bit value; so your first patch probably did<br>the right thing.<br><br>However: we should fix the actual source of the problem - that the<br>fields in struct esel_request_t are unnecessarily split into uint8_t<br>fields.<br><br>I think it'd be much better if we changed esel_request_t to this:<br><br> struct esel_request_t {<br> uint16_t resid;<br> uint16_t selrecord;<br> uint16_t offset;<br> uint8_t progress;<br> } __attribute__ ((packed)) ;<br><br>- and did the proper conversion with le16toh().<br><br>[Also, the typo in the commit title is still present]<br><br>Regards,<br><br><br>Jeremy<br><br></font></tt><br><BR>