On 06/09/18(Thu) 23:01, Michael Mikonos wrote:
> Hello,
>
> In the umidi(4) driver the function alloc_all_endpoints_fixed_ep()
> returns USBD_NOMEM if mallocarray() fails, but the return value is
> always USBD_INVAL when 'goto error' happens. OK?
Note that it'd be better to *not* use `usbd_status' for indicating
generic failures. It would be great if `usbd_status' could be only
used for code related to transfer (xfer) error/status.
In that particular case alloc_all_endpoints() & friends could use
normal errnos values. But that's a different issue ;)
Since you're here, adding sizes to free(9) is also a nice possible
cleanup :D
Anyway, your diff is ok mpi@
> Index: umidi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.47
> diff -u -p -u -r1.47 umidi.c
> --- umidi.c 6 Sep 2018 09:48:23 -0000 1.47
> +++ umidi.c 6 Sep 2018 14:28:21 -0000
> @@ -428,7 +428,6 @@ free_all_endpoints(struct umidi_softc *s
> static usbd_status
> alloc_all_endpoints_fixed_ep(struct umidi_softc *sc)
> {
> - usbd_status err;
> struct umq_fixed_ep_desc *fp;
> struct umidi_endpoint *ep;
> usb_endpoint_descriptor_t *epd;
> @@ -458,14 +457,12 @@ alloc_all_endpoints_fixed_ep(struct umid
> if (!epd) {
> DPRINTF(("%s: cannot get endpoint descriptor(out:%d)\n",
> sc->sc_dev.dv_xname, fp->out_ep[i].ep));
> - err = USBD_INVAL;
> goto error;
> }
> if (UE_GET_XFERTYPE(epd->bmAttributes)!=UE_BULK ||
> UE_GET_DIR(epd->bEndpointAddress)!=UE_DIR_OUT) {
> printf("%s: illegal endpoint(out:%d)\n",
> sc->sc_dev.dv_xname, fp->out_ep[i].ep);
> - err = USBD_INVAL;
> goto error;
> }
> ep->sc = sc;
> @@ -485,14 +482,12 @@ alloc_all_endpoints_fixed_ep(struct umid
> if (!epd) {
> DPRINTF(("%s: cannot get endpoint descriptor(in:%d)\n",
> sc->sc_dev.dv_xname, fp->in_ep[i].ep));
> - err = USBD_INVAL;
> goto error;
> }
> if (UE_GET_XFERTYPE(epd->bmAttributes)!=UE_BULK ||
> UE_GET_DIR(epd->bEndpointAddress)!=UE_DIR_IN) {
> printf("%s: illegal endpoint(in:%d)\n",
> sc->sc_dev.dv_xname, fp->in_ep[i].ep);
> - err = USBD_INVAL;
> goto error;
> }
> ep->sc = sc;
> @@ -509,7 +504,7 @@ alloc_all_endpoints_fixed_ep(struct umid
> error:
> free(sc->sc_endpoints, M_USBDEV, 0);
> sc->sc_endpoints = NULL;
> - return err;
> + return USBD_INVAL;
> }
>
> static usbd_status
>
