On Sat, 13 Mar 2010, Bruno Prémont wrote: > > If the device_add() fails, you should undo the hid_debug_register() > > call. > > Well I was wondering why hid_debug_register() was called no matter what > device_add() returned but didn't dig around what happened when > hid_add_device() would return an error. > > Looking a few lines further, hid_remove_device() just unregisters debugfs > entries when status has HID_STAT_ADDED so it definitely makes sense to > do the proper cleanup (and hid_remove_device() is the only once calling > hid_debug_unregister()) > > So patch for both below: > > > > > Register debugfs entries before calling device_add() so debugfs entries > are already present when HID driver's probe function gets called on device > hotplug. > Also undo debugfs entry registration if device_add() fails so status > HID_STAT_ADDED and debugfs registration status remain consistent and > we don't leak the debugfs entries. > > Signed-off-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > --- > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index eabe5f8..709b4d0 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1759,11 +1759,12 @@ int hid_add_device(struct hid_device *hdev) > dev_set_name(&hdev->dev, "%04X:%04X:%04X.%04X", hdev->bus, > hdev->vendor, hdev->product, atomic_inc_return(&id)); > > + hid_debug_register(hdev, dev_name(&hdev->dev)); > ret = device_add(&hdev->dev); > if (!ret) > hdev->status |= HID_STAT_ADDED; > - > - hid_debug_register(hdev, dev_name(&hdev->dev)); > + else > + hid_debug_unregister(hdev); > > return ret; Applied, thank you Bruno. -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
