Re: [PATCH] watchdog: add driver for StreamLabs USB watchdog device

From: Oliver Neukum
Date: Wed Mar 16 2016 - 05:10:49 EST


On Wed, 2016-03-16 at 00:57 +0000, Alexey Klimov wrote:
> Hi Oliver,
>
> On Thu, Mar 10, 2016 at 9:23 AM, Oliver Neukum <oneukum@xxxxxxxx> wrote:
> > On Thu, 2016-03-10 at 02:29 +0000, Alexey Klimov wrote:

> >> + streamlabs_wdt->buffer[3] = 0x80;
> >> +
> >> + streamlabs_wdt->buffer[6] = (timeout_msec & 0xff) << 8;
> >> + streamlabs_wdt->buffer[7] = (timeout_msec & 0xff00) >> 8;
> >
> > We have macros for such conversions. Please use them.
>
> I screwed here. It should be:
> buffer[6] = timeout_msec & 0xff;
> buffer[7] = (timeout_msec >> 8) & 0xff;
>
> However, are you talking about using swab16() function? Or migrating
> to cpu_to_le() and friends functions?
>
> If it's acceptable way to make buffer with u16 type?
> It slightly decreases lines of code and no conversion is needed here
> that way. I can do just without swapping bytes:
> buffer[3] = timeout_msec;
> and that's it.

NO!

You cannot make assumptions about the byte order of the host. This goes
over the wire. It must be in a defined order. You need to use the
cpu_to_* family of macros which will do the right thing. You just
open coded them.

Sorry to be very blunt here, but we absolutely must not make assumptions
about which hosts USB drivers can run on.

HTH
Oliver