I think I agree with David here... but I'm having so much trouble understanding both people that it's hard to be sure.
On Wed, 24 Sep 2003, David Brownell wrote: > Greg KH wrote: > > > > Try it :) From what I recall, we have to increment the count at this > > point, as we increment it and decrement it when we start talking to the > > device (getting the config and stuff.) This is because when we submit a > > urb, we increment the device's count. I don't understand this comment at all. We have to increment the count here because later on we will increment it again? That can't be right. Unless some later part of the code is decrementing the counter _before_ incrementing it (a clear error), there should not be any need for the extra increment in usb_new_device(). > That refcount bug (now fixed) in usb_new_device() error paths could > have benefited (oopses vanishing) from having an extra refcount. > I could believe those problems were covering up for each other; > they were right in the same area, and neither made sense otherwise. Do you mean that the apparent reason for the extra increment was to cover up for some other error which has since been fixed? > The only reason I can see to have an extra refcount in that path is > in case the counted pointer (hub->children[]) gets freed somehow > while there's still another copy of that pointer, used on the stack. If the pointer is freed, how would the fact that the device it used to point to had a positive refcount help anything? Do you really mean that some code might be doing an unneeded decrement somewhere -- causing it to free the pointer too early -- and that the extra increment is meant to prevent that? > Otherwise the normal "shared lifetime" rules apply ... and even > in that case, the extra refcount should be dropped when the copy > from the stack goes away. Exactly what do you mean by "shared lifetime"? Are you referring to a possible problem in which the device is unplugged right away, so usb_disconnect() gets called while usb_new_device() is still running? To prevent that from messing things up, the extra usb_get_dev() in new_device() would have to be balanced by a usb_put_dev() when new_device() returns, not in usb_disconnect(). Is that the point you want to make? But this scenario shouldn't be possible, given the way that khubd works. > When a GET_DESCRIPTOR (or SET_ADDRESS, or SET_CONFIGURATION, etc) > request gets made, there's a temporary extra refcount that's only > maintained during the invocation path. It's transparent, and it > goes back (for example, down to one) when the call returns. From > the perspective of the synchronous usb_new_device() logic, it won't > matter: that code already has a valid refcount. Yes. Alan Stern ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
