[PATCH openpower-host-ipmi-oem v3] Fix issue5 for endian safe
Jeremy Kerr
jk at ozlabs.org
Mon May 30 16:06:59 AEST 2016
Hi Nan,
> @@ -29,9 +30,8 @@ ipmi_ret_t ipmi_ibm_oem_partial_esel(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
> esel_request_t *reqptr = (esel_request_t*) request;
> FILE *fp;
> int r = 0;
> - // TODO: Issue 5: This is not endian-safe.
> - short *recid = (short*) &reqptr->selrecordls;
> - short *offset = (short*) &reqptr->offsetls;
> + short recid = le16toh((((unsigned short) reqptr->selrecordms) << 8) + reqptr->selrecordls);
> + short offset = le16toh((((unsigned short) reqptr->offsetms) << 8) + reqptr->offsetls);
I think that this is less correct than the first attempt, and that's
probably due to an incorrect suggestion from me. Sorry for that.
I've only just noticed that you're using different members of the reqptr
struct to construct that 16-bit value; so your first patch probably did
the right thing.
However: we should fix the actual source of the problem - that the
fields in struct esel_request_t are unnecessarily split into uint8_t
fields.
I think it'd be much better if we changed esel_request_t to this:
struct esel_request_t {
uint16_t resid;
uint16_t selrecord;
uint16_t offset;
uint8_t progress;
} __attribute__ ((packed)) ;
- and did the proper conversion with le16toh().
[Also, the typo in the commit title is still present]
Regards,
Jeremy
More information about the openbmc
mailing list