PERFORCE change 170842 for review
Oliver Fromme
olli at fromme.com
Fri Nov 20 08:53:24 UTC 2009
Nathan Whitehorn wrote:
> Hans Petter Selasky wrote:
> > http://p4web.freebsd.org/chv.cgi?CH=170842
> >
> > Change 170842 by hselasky at 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)
More information about the p4-projects
mailing list