From owner-p4-projects@FreeBSD.ORG Fri Nov 20 08:53:24 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 718EB1065672; Fri, 20 Nov 2009 08:53:24 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 35EBE106566C for ; Fri, 20 Nov 2009 08:53:24 +0000 (UTC) (envelope-from olli@fromme.com) Received: from haluter.fromme.com (haluter.fromme.com [212.17.241.231]) by mx1.freebsd.org (Postfix) with ESMTP id B14A88FC26 for ; Fri, 20 Nov 2009 08:53:23 +0000 (UTC) Received: from haluter.fromme.com (irc_sucks@localhost [127.0.0.1]) by haluter.fromme.com (8.14.3/8.14.3) with ESMTP id nAK8rFqw012641; Fri, 20 Nov 2009 09:53:22 +0100 (CET) (envelope-from olli@fromme.com) Received: (from olli@localhost) by haluter.fromme.com (8.14.3/8.14.3/Submit) id nAK8rEhF012639; Fri, 20 Nov 2009 09:53:14 +0100 (CET) (envelope-from olli) From: Oliver Fromme Message-Id: <200911200853.nAK8rEhF012639@haluter.fromme.com> To: nwhitehorn@freebsd.org (Nathan Whitehorn) Date: Fri, 20 Nov 2009 09:53:14 +0100 (CET) In-Reply-To: <4B05CB1F.8020100@freebsd.org> X-Mailer: ELM [version 2.5 PL8] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.1.2 (haluter.fromme.com [127.0.0.1]); Fri, 20 Nov 2009 09:53:22 +0100 (CET) Cc: Perforce Change Reviews , Hans Petter Selasky Subject: Re: PERFORCE change 170842 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Nov 2009 08:53:24 -0000 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)