Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Nov 2009 09:11:27 -0600
From:      Nathan Whitehorn <nwhitehorn@freebsd.org>
To:        Hans Petter Selasky <hselasky@freebsd.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 170842 for review
Message-ID:  <4B06B19F.7050501@freebsd.org>
In-Reply-To: <200911200846.54559.hselasky@freebsd.org>
References:  <200911192235.nAJMZ2XH072195@repoman.freebsd.org> <4B05CB1F.8020100@freebsd.org> <200911200846.54559.hselasky@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hans Petter Selasky wrote:
> On Thursday 19 November 2009 23:47:59 Nathan Whitehorn wrote:
>   
>> Hans Petter Selasky wrote:
>>     
>>> http://p4web.freebsd.org/chv.cgi?CH=170842
>>>
>>> Change 170842 by hselasky@hselasky_laptop001 on 2009/11/19 22:34:49
>>>
>>>
>>> 	USB input:
>>> 		 - ATP patch from Rohit Grover:
>>> 		   - fixes some minor issues and
>>> 		     makes the control transfer
>>> 		     fully asynchronous
>>>       
>> [...]
>>
>>     
>>> @@ -1530,7 +1574,7 @@
>>>  		return (ENXIO);
>>>
>>>  	if (usbd_lookup_id_by_uaa(atp_devs, sizeof(atp_devs), uaa) == 0)
>>> -		return BUS_PROBE_SPECIFIC;
>>> +		return 0;
>>>  	else
>>>  		return ENXIO;
>>>  }
>>>       
>> Why are you replacing symbolic constants with less informative numeric
>> ones? -Nathan
>>     
>
> Because returning zero in probe has special meaning and is hardcoded in the 
> subr_bus.c code aswell. The other return values will not be changed.
>   
It's the same thing as far as the code is concerned, of course, my 
complaint was merely a style issue. Using the constant makes the meaning 
of the return value clearer, especially since this driver uses this 
return value to override the BUS_PROBE_GENERIC priority of ums(4). 
Changing it from the constant that was already there seemed like a step 
backward in readability.
-Nathan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B06B19F.7050501>