cvs commit: src/sys/dev/ata ata-all.c

[Scott Long]

M. Warner Losh wrote:
> In message: <20080421233847.GM82555 at funkthat.com>
>             John-Mark Gurney <jmg at 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