Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Nov 2009 12:22:23 +0100
From:      Hans Petter Selasky <hselasky@freebsd.org>
To:        Oliver Fromme <olli@fromme.com>
Cc:        Perforce Change Reviews <perforce@freebsd.org>, Nathan Whitehorn <nwhitehorn@freebsd.org>
Subject:   Re: PERFORCE change 170842 for review
Message-ID:  <200911201222.25052.hselasky@freebsd.org>
In-Reply-To: <200911200853.nAK8rEhF012639@haluter.fromme.com>
References:  <200911200853.nAK8rEhF012639@haluter.fromme.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 20 November 2009 09:53:14 Oliver Fromme wrote:
> 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).

Fixed.

--HPS




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