From owner-cvs-src@FreeBSD.ORG Tue Apr 22 13:22:46 2008 Return-Path: Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D83F21065673; Tue, 22 Apr 2008 13:22:46 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.freebsd.org (Postfix) with ESMTP id 632658FC1B; Tue, 22 Apr 2008 13:22:46 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from phobos.samsco.home (phobos.samsco.home [192.168.254.11]) (authenticated bits=0) by pooker.samsco.org (8.13.8/8.13.8) with ESMTP id m3MDMbHd076763; Tue, 22 Apr 2008 07:22:38 -0600 (MDT) (envelope-from scottl@samsco.org) Message-ID: <480D3DDC.2050500@samsco.org> Date: Mon, 21 Apr 2008 19:22:36 -0600 From: Scott Long User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9 MIME-Version: 1.0 To: "M. Warner Losh" References: <20080421213724.GL82555@funkthat.com> <480D0E44.9070201@samsco.org> <20080421233847.GM82555@funkthat.com> <20080421.213858.932034124.imp@bsdimp.com> In-Reply-To: <20080421.213858.932034124.imp@bsdimp.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-0.9 required=3.8 tests=ALL_TRUSTED,AWL, DATE_IN_PAST_12_24 autolearn=ham version=3.1.8 X-Spam-Checker-Version: SpamAssassin 3.1.8 (2007-02-13) on pooker.samsco.org 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Apr 2008 13:22:47 -0000 M. Warner Losh wrote: > In message: <20080421233847.GM82555@funkthat.com> > John-Mark Gurney 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