Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c

["Mitsuhiro KOGA" <shiena.jp@gmail.com>]

I'm sorry. Because the mistake and lack were found in the explanation,
a postscript is added to it.

2006/8/17, Mitsuhiro KOGA <shiena.jp@gmail.com>:
> 2006/8/16, Antonino A. Daplas <adaplas@gmail.com>:
> > On Wed, 2006-08-16 at 20:31 +0900, Mitsuhiro KOGA wrote:
> > > Hi,
> > >
> >
> > >  static void
> > >  sisusb_free_buffers(struct sisusb_usb_data *sisusb)
> > >  {
> > > @@ -408,7 +465,11 @@ static int sisusb_send_bulk_msg(struct s
> > >
> > >               } else if (fromkern) {
> > >
> > > -                     memcpy(buffer, kernbuffer, passsize);
> > > +                     if ((len & 3) == 0)
> > > +                             sisusb_memcpy((u16 *)buffer, (u16 *)kernbuffer, passsize / 2);
> > > +                     else
> > > +                             memcpy(buffer, kernbuffer, passsize);
> >
> > What happens if (len & 3) != 0? It ends up doing a regular memcpy()
> > without the reordering.  Better if you move the if (len & 3) == 0 check
> > to sisusb_memcpy(), so instead of
> >
> >                        if ((len & 3) == 0)
> >                                sisusb_memcpy((u16 *)buffer, (u16 )kernbuffer, passsize / 2);
> >                        else
> >                                memcpy(buffer, kernbuffer, passsize);
> >
>
> (a) if (len & 3) != 0
> When former data length is 4 or less, struct sisusb_packet is used.
> It becomes 10 in length for writing.
> Because data is not used, it becomes 6 in length for reading.
>
> struct sisusb_packet {
>     unsigned short header;
>     u32 address;
>     u32 data;
> } __attribute__((__packed__));
>
> (b) if (len & 3) == 0
> When former data length is larger than 4,
> it corresponds to this case because the length of (data & ~3) is passed.
> As for the data of the remainder, the logic of (a) is used.

The length passed here is not (data & ~3) but (former data length & ~3).

And, variable buffer becomes a character string displayed in the monitor
a couple of byte sequence by two bytes (the character-code and the delimiter).
Because this order reverses for the Big endian, variable buffer is
being replaced.

In the case of (a), sisusb_pack->data is being replaced with
cpu_to_le32 in existing processing,
and because the order of the character reverses for four bytes, data
is being replaced
with sisusb_order_mem_32 further.

Regards,

-- 
Mitsuhiro KOGA
<shiena.jp@gmail.com>

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642