Re: cleanup of hiddev
From: Jiri Slaby <hidden>
Date: 2008-11-03 20:05:13
Oliver Neukum napsal(a):
Hi Jiřis,
:) Hi.
this is the cleanup of hiddev against current vanilla. What do you think?
See my comments below.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index 3ac3207..78f5a3b 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c@@ -317,8 +325,8 @@ static ssize_t hiddev_read(struct file * file, char __user * buffer, size_t coun while (retval == 0) { if (list->head == list->tail) { - add_wait_queue(&list->hiddev->wait, &wait); set_current_state(TASK_INTERRUPTIBLE); + add_wait_queue(&list->hiddev->wait, &wait);
maybe better to convert to prepare_to_wait()?
while (list->head == list->tail) {But moved here (or the set_current_state).
quoted hunk ↗ jump to hunk
if (file->f_flags & O_NONBLOCK) {@@ -335,7 +343,6 @@ static ssize_t hiddev_read(struct file * file, char __user * buffer, size_t coun } schedule(); - set_current_state(TASK_INTERRUPTIBLE);
This is wrong. Schedule ensures state to be TASK_RUNNING. Not setting it again for the next loop would be a bug (but better to be done right after the while statement).
} set_current_state(TASK_RUNNING);
and finish_wait()
quoted hunk ↗ jump to hunk
@@ -810,13 +817,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force) if (!(hiddev = kzalloc(sizeof(struct hiddev), GFP_KERNEL))) return -1; - retval = usb_register_dev(usbhid->intf, &hiddev_class); - if (retval) { - err_hid("Not able to get a minor for this device."); - kfree(hiddev); - return -1; - } - init_waitqueue_head(&hiddev->wait); INIT_LIST_HEAD(&hiddev->list); spin_lock_init(&hiddev->list_lock);@@ -828,6 +828,14 @@ int hiddev_connect(struct hid_device *hid, unsigned int force) hiddev_table[usbhid->intf->minor - HIDDEV_MINOR_BASE] = hiddev;
usbhid->intf->minor is set even after this call:
+ retval = usb_register_dev(usbhid->intf, &hiddev_class);
+ if (retval) {
+ err_hid("Not able to get a minor for this device.");
+ hiddev_table[usbhid->intf->minor - HIDDEV_MINOR_BASE] = NULL;
+ kfree(hiddev);
+ return -1;
+ }
+
return 0;
}
-- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html