On Mon, May 14, 2012 at 09:18:38AM -0700, Greg KH wrote: > On Mon, May 14, 2012 at 11:25:29PM +0800, Lan Tianyu wrote: > > Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx> > > --- > > drivers/usb/core/hub.c | 14 ++++++++++++++ > > 1 files changed, 14 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 6bf7124..6f909db 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -4236,6 +4236,20 @@ void usb_queue_reset_device(struct usb_interface *iface) > > } > > EXPORT_SYMBOL_GPL(usb_queue_reset_device); > > > > +/** > > + * usb_get_hub_child_device - Get the pointer of child device > > + * attached to the port which is specified by @port1. > > + * > > + * @hdev: USB device belonging to the usb hub > > + * @port1: port num to indicate which port the child device > > + * is attached to. > > + * > > + * USB drivers call this function to get hub's child device > > + * pointer. > > + * > > + * Return NULL if input param is invalid and > > + * child's usb_device pointer if non-NULL. > > + */ > > struct usb_device *usb_get_hub_child_device(struct usb_device *hdev, > > The more I look at this function, the worse I like it, here's why: > - you don't increment the reference count of the device, what is to > keep it from going away from when you find it and the caller uses > it? > - why all of the checking of the arguments, how, and who, would ever > mess up calling this function incorrectly? > - Whenever you are using this function, it's in a loop, which > indicates that what you really want is a function to iterate over > the child devices of a hub, right? So that is the function you need > to write, which will handle the proper locking and incrementing of > device reference counts. > > So, care to just delete the function and do this correctly instead? As > it is, it seems very fragile. Because of this, I've reverted the patch that added this function, and the one prior to this, as this series really isn't that thought out just yet, and I don't want it into 3.5. Please take the time to rework these patches and do it correctly. thanks, greg k-h -- 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
