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?
That's my suspicion. The details of the refcounting in usbcore never made sense to me otherwise. Getting a refcount should be closely coupled with storing a new copy of a pointer; and dropping one should be coupled with clearing such a copy. But there have always been cases (like these!) where usbcore hasn't adopted that simple provably-correct strategy.
That is, I _think_ the other bug is one I fixed a while back where usb_new_device() miscounted its references when devices didn't SET_ADDRESS or had problems with GET_DESCRIPTOR. But maybe there were other problems; it's worth fixing refcount problems very cautiously.
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?
That's a "cover up for some other error" kind of scenario.
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
Go to anything that describes refcounting rules very carefully (like for MSFT's COM, which was nowhere near the first but is at least widely available) and you'll see something analagous.
Basically, if you hold a refcount, and make a synchronous call to something else, then the code you call doesn't need to manage another refcount all of its own: your refcount suffices. If it manages one, that's fine -- a NOP. But you certainly don't need to manage an extra refcount on its behalf; that'd just be pointless overhead.
possible problem in which the device is unplugged right away, so usb_disconnect() gets called while usb_new_device() is still running? To
That's the scenario I was sketching, yes.
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.
True. My comment was basically "suppose that it shouldn't be possible, but there's a bug so it is ... even then, this code would not be managing the refcount right."
- Dave
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
