On Mon, Mar 08, 2010 at 10:13:49PM -0800, Ping Cheng wrote: > On Mon, Mar 8, 2010 at 9:43 PM, Peter Hutterer > <peter.hutte...@who-t.net>wrote: > > > I think I may have it now, thanks much for the pointers. The patch is > > attached but it also needs a revert of the "Remove superfluous > > packet-length > > assignment" patch on the isdv4 branch. > > > > I'm a bit confused though, in the original code, all data was read into a > > buffer, then the first byte of this buffer was checked and the > > packet length decided on. Then the code would read as many packets as > > viable > > from this buffer, until the remainder was < pktlen. I'm a bit confused on > > how this code handled packets on different lengths though since it'd always > > check the first byte in the buffer, never of the actual packet. > > > > A valid data packet has one of the following format: > > Pen: > > 1 x x x x x x x > 0 x x x x x x x (seven more bytes of this format) > > 1FGT > 1 x x x x x x x > 0 x x x x x x x (three more bytes of this format) > > 2FGT > 1 x x x x x x x > 0 x x x x x x x (eleven more bytes of this format) > > So, you need to decide what kind of data you are getting to set the pktlen > first. Then, verify the packet with first bit of the first byte, it has to > be 1. If it is correct, read the next pktlen-1 bytes, all first bits have > to be 0. If not, start a new round of validation process with the next byte > that the first bit equals to 1. If there is not enough bytes in the packet, > keep it for the next read and wait for the next packet to come.
ok. I had a look again - what confused me was that 59754d40db384f0d8cd88 removed a bunch of checks where the loop would exit if the data length changes. given those checks (and the move of the byte up to the first position) it makes sense to only check the first byte for information. in current xf86-input-wacom master though, we only ever check the first byte but keep looping through the packets anyway. so with the first packet of different length, this goes out of sync. linuxwacom still has these checks in which might well be the reason that we see some weird issues in xf86-input-wacom with serial devices that we don't see in linuxwacom. anyhow, this _should_ be fixed by the one-liner patch to check the first byte of the buffer passed into Parse() since that byte now moves and we adjust the length accordingly. > With the devel tree, I think the issue is with one line - that we only > > checked the first byte of the buffer as well but not of the data passed in. > > Given that each time we get data this is supposed to be a whole packet we > > can assume that the first byte contains the info we need. does this make > > sense? > > > > You can not assume there is always a whole packet. You could get a partial > packet. The first byte you get in a packet doesn't count. The one that > starts with data[*] & 0x80 != 0 counts the first valid byte of a data set. ok, I'll add some more checks to make sure the validation code does what it's supposed to. Cheers, Peter ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
