On Fri, Jul 30, 2010 at 03:08:42PM +0400, Kulikov Vasiliy wrote: > diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c > index f58da32..57f4946 100644 > --- a/drivers/staging/panel/panel.c > +++ b/drivers/staging/panel/panel.c > @@ -1589,25 +1589,30 @@ void lcd_init(void) > static ssize_t keypad_read(struct file *file, > char *buf, size_t count, loff_t *ppos) > { > - > + int buflen = keypad_buflen; > unsigned i = *ppos; > char *tmp = buf; > + int start = keypad_start; > > - if (keypad_buflen == 0) { > + if (buflen == 0) { > if (file->f_flags & O_NONBLOCK) > return -EAGAIN; > > interruptible_sleep_on(&keypad_read_wait); > if (signal_pending(current)) > return -EINTR; > + buflen = keypad_buflen; > } Not sure what the intent was here, I think you're re-adjusting the buffer's length in case something else was read. But then I don't understand why buflen it not simply assigned after the if() block. The rest below looks fine otherwise. > > - for (; count-- > 0 && (keypad_buflen > 0); > - ++i, ++tmp, --keypad_buflen) { > - put_user(keypad_buffer[keypad_start], tmp); > - keypad_start = (keypad_start + 1) % KEYPAD_BUFFER; > + for (; count-- > 0 && (buflen > 0); > + ++i, ++tmp, --buflen) { > + if (put_user(keypad_buffer[start], tmp)) > + return -EFAULT; > + start = (start + 1) % KEYPAD_BUFFER; > } > *ppos = i; > + keypad_buflen = buflen; > + keypad_start = start; > > return tmp - buf; > } Regards, Willy _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel