Date: Fri, 20 Nov 2009 09:53:14 +0100 (CET) From: Oliver Fromme <olli@fromme.com> To: nwhitehorn@freebsd.org (Nathan Whitehorn) Cc: Perforce Change Reviews <perforce@freebsd.org>, Hans Petter Selasky <hselasky@freebsd.org> Subject: Re: PERFORCE change 170842 for review Message-ID: <200911200853.nAK8rEhF012639@haluter.fromme.com> In-Reply-To: <4B05CB1F.8020100@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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? As far as I can see, the change makes sense. The function atp_probe() returns 0 on success, or an errno value if an error occurs, but BUS_PROBE_SPECIFIC is not an errno symbol, and there is no symbolic constant for the errno value 0, according to intro(2), so it's appropriate to use the numeric constant 0. Many kernel functions do that. However, it could be argued that a better way might be to define your own error symbol space, like USB_SUCCESS, USB_ERROR or possibly others, and translate to proper errno values only where necessary. Several kernel sub- systems do this. By the way, style(9) states that return values should always be put in parentheses, even though the C standard doesn't require it. So it should be return (0). Best regards Oliver -- Oliver Fromme, Bunsenstr. 13, 81735 Muenchen, Germany ``We are all but compressed light'' (Albert Einstein)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911200853.nAK8rEhF012639>