On Tue, 31 Jul 2007, David Brownell wrote: > On Tuesday 31 July 2007, Alan Stern wrote: > > > Related: consider making urb->interval and its neighbors > > > be "u32" or maybe even "u16". > > > > Hmmm... maybe. It's not clear the space savings would matter much; I > > doubt that terribly many URBs ever get allocated at the same time. > > I don't follow. Space savings are more often an accumulation of > small improvements than any single big thing.
That may be. But shrinking fields like this can cause code size to increase, because the instructions needed for accessing the smaller fields tend to be longer (and slower!) than the instructions for accessing a normal word. There needs to be a balance. > And allocating many > at once vs the same number over time doesn't matter, either ... the > same amount of memory would be saved. Not so. If you allocated 100 URBs at the same time, the space savings would be multiplied by a factor of 100. But if you allocated 10 URBs and deallocated them, then allocated another 10 and deallocated them, and so on 10 times, the space savings would be multiplied by a factor of only 10, not 100. > > And > > while it's true that the values should never be negative, there's not > > much incentive to change the type to unsigned. > > I'll just disagree. Code written so that it's not *possible* to > have a class of errors is better than similar code which could > misbehave. I agree. But in this case the code has already been written and you're suggesting a change -- which might introduce its own bugs. > In this case, you agree "should never be negative", > but evidently think there might be some benefit to letting those > values become negative so that algorithms could execute with > data outside of the intended and tested ranges ... really, I > can't follow that logic. Well, consider this example. urb->actual_length is naturally non-negative, right? But uhci-hcd initializes it to -8 for control URBs, so that the effect of accumulating the transfer size of the setup packet is automatically taken into account. There's a little extra code to prevent negative values from ever becoming visible to drivers in cases where the setup packet isn't transmitted; that extra code would become buggy if actual_length were changed to an unsigned type. Sure, it could be fixed. The point is that without careful checking, merely changing a signed integer to unsigned can introduce bugs. So let's put it like this: If _you_ would like to change those fields, I won't stand in your way. I'll even go through uhci-hcd and dummy-hcd to make sure there are no ill effects. An even better change would be to remove urb->error_count. IIRC it is used only in a couple of drivers, and there just for debug log messages. Alan Stern ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
