usbtouchscreen: version 0.4

Hi Daniel,

I had a question about patch 5d8926658ce4: "usbtouchscreen: version 0.4" from Jul 31, 2006.

drivers/input/touchscreen/usbtouchscreen.c
1326 static void usbtouch_process_multi(struct usbtouch_usb *usbtouch,
1327 unsigned char *pkt, int len)
1328 {
1329 unsigned char *buffer;
1330 int pkt_len, pos, buf_len, tmp;
1331
1332 /* process buffer */
1333 if (unlikely(usbtouch->buf_len)) {
1334 /* try to get size */
1335 pkt_len = usbtouch->type->get_pkt_len(
1336 usbtouch->buffer, usbtouch->buf_len);

->get_pkt_len() returns -1 on error.

  1337  
  1338                  /* drop? */
  1339                  if (unlikely(!pkt_len))
  1340                          goto out_flush_buf;
  1341  
  1342                  /* need to append -pkt_len bytes before able to get size */
  1343                  if (unlikely(pkt_len < 0)) {
  1344                          int append = -pkt_len;

But then we treat the -1 as a length of 1. Is that really what we intended?

  1345                          if (unlikely(append > len))
  1346                                 append = len;
  1347                          if (usbtouch->buf_len + append >= usbtouch->type->rept_size)
  1348                                  goto out_flush_buf;
  1349                          memcpy(usbtouch->buffer + usbtouch->buf_len, pkt, append);
  1350                          usbtouch->buf_len += append;
  1351  
  1352                          pkt_len = usbtouch->type->get_pkt_len(
  1353                                          usbtouch->buffer, usbtouch->buf_len);
  1354                          if (pkt_len < 0)
  1355                                  return;
  1356                  }
  1357  
  1358                  /* append */
  1359                  tmp = pkt_len - usbtouch->buf_len;

regards,
dan carpenter

Hi Dan

That's ancient :)

No, it doesn't and never has. In 62aa366d9b015 I added a bit of documentation:

/*
* used to get the packet len. possible return values:
* > 0: packet len
* = 0: skip one byte
* < 0: -return value more bytes needed
*/
int (*get_pkt_len) (unsigned char *pkt, int len);

So -1 means "need one more byte for a complete data packet". That byte will be in delivered in the next invocation of usbtouch_irq(). One "touchscreen data packet" can be split across multiple "usb packets" for some devices.

regards
-Daniel

Ah, I'm amazed your still responding to emails about this ancient driver. :) Thanks for the explanation.

regards,
dan carpenter