On Wednesday 09 February 2005 11:01 am, Nishanth Aravamudan wrote: > On Wed, Feb 09, 2005 at 10:33:02AM -0800, David Brownell wrote: > > On Tuesday 08 February 2005 10:05 pm, Nishanth Aravamudan wrote: > > > - USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, HZ); > > > + USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, 1000); > > > > You know, changing from symbols to magic numbers is not a win. > > People know what something involving HZ is intended to mean, > > but 1000 is just another magic number ... > > To be honest, I had a similar concern. But if the API is clear that the > timeout parameter is in real time units (milliseconds, in this case), I > do not think 1000 is as much of a "magic number" as HZ was, or > minimally, it's the same.
You mean you can actually tell, from looking at a large set of function parameters, which ones indicate timeouts? Wow! Most folk I know usually deduce that it's the one that involves HZ, MSEC, or USEC ... or, it's the single parameter to a *sleep function. :) For maintainability, code needs to have a few basic accommodations for newcomers -- or semi-forgetful oldtimers. (I think that covers pretty much every developer, come to think of it...) One of those accomodations is using symbolic constants. > > Rather than 1000 how about something like "1 * MS_PER_SEC" or something > > similar? I especially don't like to see magic numbers added to usbcore, > > but the same issue applies to a lot of drivers too. > > *sigh* That would mean sending out all ~63 patches again, but so it > goes. If Greg would prefer it this way, I'd be happy to comply. One common practice is to send out core patches plus a few example "here's how the drivers would change" patches, purely for comments. (Especially if, as you say, you have concerns; they might be shared by other folk.) Then wait a few days (idealy a week or so) to collect feedback before submitting the patches. Whenever you send out a large batch of patches, or a set of related complex patched, it's best to have at least tried to collect feedback beforehand ... to avoid exactly that problem. - Dave > -Nish > ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
