Date: Mon, 21 Apr 2008 19:22:36 -0600 From: Scott Long <scottl@samsco.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-src@FreeBSD.org, jmg@funkthat.com, bz@FreeBSD.org, cvs-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/ata ata-all.c Message-ID: <480D3DDC.2050500@samsco.org> In-Reply-To: <20080421.213858.932034124.imp@bsdimp.com> References: <20080421213724.GL82555@funkthat.com> <480D0E44.9070201@samsco.org> <20080421233847.GM82555@funkthat.com> <20080421.213858.932034124.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
M. Warner Losh wrote: > In message: <20080421233847.GM82555@funkthat.com> > John-Mark Gurney <jmg@funkthat.com> writes: > : Scott Long wrote this message on Mon, Apr 21, 2008 at 15:59 -0600: > : > John-Mark Gurney wrote: > : > >Bjoern A. Zeeb wrote this message on Sun, Apr 20, 2008 at 17:45 +0000: > : > >>bz 2008-04-20 17:45:32 UTC > : > >> > : > >> FreeBSD src repository > : > >> > : > >> Modified files: > : > >> sys/dev/ata ata-all.c > : > >> Log: > : > >> devclass_get_maxunit() returns n+1 with n starting at 0. > : > >> So if we have channel 0..3 devclass_get_maxunit is 4. > : > >> > : > >> It's never been a problem as devclass_get_device() has > : > >> catched a possibly bad input. > : > > > : > >Any one object to changing: > : > >.Nm devclass_get_maxunit > : > >.Nd find the maximum unit number in the class > : > > > : > >to: > : > >.Nm devclass_get_maxunit > : > >.Nd find the next free unit number in the class > : > > : > That's not what it actually returns though. It returned the highest > : > allocated unit number plus 1. The unit numbering can be sparse, with > : > the next available unit number being less than the highest allocated > : > unit number. > : > : Yeh, that was partly about changing the description... Can you think of > : a better name besides devclass_get_maxunitplusone? > : > : > Most callers use this value as the limit in a for loop, hence why it's > : > convenient for it to return the +1. > : > : Yeh, but it definately does not return maxunit.. :) unitarraysize? > : > : Hmmm... find isn't a useful verb, since it doesn't do any finding... > : it returns a stored value... How about: > : .Nd return the max number of units in the class > : > : And then flush out the description about using it for an array? Though > : it doesn't solve the naming issue... > > Well, there already is: > >>> This is one greater than the highest currently allocated unit. > > in the man page. Consider the following diff: > > Index: devclass_get_maxunit.9 > =================================================================== > RCS file: /home/ncvs/src/share/man/man9/devclass_get_maxunit.9,v > retrieving revision 1.8 > diff -u -r1.8 devclass_get_maxunit.9 > --- devclass_get_maxunit.9 28 Jun 2005 20:15:18 -0000 1.8 > +++ devclass_get_maxunit.9 22 Apr 2008 03:37:47 -0000 > @@ -33,16 +33,22 @@ > .Os > .Sh NAME > .Nm devclass_get_maxunit > -.Nd find the maximum unit number in the class > +.Nd finds a number larger than any allocated unit > .Sh SYNOPSIS > .In sys/param.h > .In sys/bus.h > .Ft int > .Fn devclass_get_maxunit "devclass_t dc" > .Sh DESCRIPTION > -Returns the next unit number to be allocated to device instances in the > +Returns a number greater than the highest allocated unit for this > .Dv devclass . > This is one greater than the highest currently allocated unit. > +Loops may use this number as an upper bound for getting the > +.Dv device > +associated with the nth unit of > +.Dv devclass . > +A new unit allocated for this device will not necessarily be the > +returned value of this function. > .Sh SEE ALSO > .Xr devclass 9 , > .Xr device 9 > @@ -51,3 +57,4 @@ > .An Doug Rabson . > .Sh BUGS > The name is confusing since it is one greater than the maximum unit. > + > > > I don't think we should rename it, however. I don't believe the gain > will be worth the MFC hassles it will cause. > > Warner Agreed. Renaming the function would be a gross overreaction to a very minor problem is that is easily solved with better documentation. Scott
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?480D3DDC.2050500>