On Tue, May 07, 2019 at 10:42:58AM -0400, Alan Stern wrote:
> On Tue, 7 May 2019, Johan Hovold wrote:
> > > @@ -411,6 +412,23 @@ static int smsusb_init_device(struct usb
> > > dev->udev = interface_to_usbdev(intf);
> > > dev->state = SMSUSB_DISCONNECTED;
> > >
> > > + for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> > > + struct usb_endpoint_descriptor *desc =
> > > + &intf->cur_altsetting->endpoint[i].desc;
> > > +
> > > + if (desc->bEndpointAddress & USB_DIR_IN) {
> > > + dev->in_ep = desc->bEndpointAddress;
> > > + in_maxp = usb_endpoint_maxp(desc);
> > > + } else {
> > > + dev->out_ep = desc->bEndpointAddress;
> > > + }
> > > + }
> > > +
> > > + pr_debug("in_ep = %02x, out_ep = %02x\n",
> > > + dev->in_ep, dev->out_ep);
> > > + if (!dev->in_ep || !dev->out_ep) /* Missing endpoints? */
> > > + return -EINVAL;
> >
> > Looks like you're now leaking dev here, and so is the current code in
> > the later error paths.
> >
> > Since this return value will be returned from probe, you may want to use
> > -ENXIO or -ENODEV instead of -EINVAL.
> >
> > Looks good otherwise.
>
> Thanks for the review. You're right about the memory leak (although
> you're wrong about the later error paths: smsusb_term_device()
> deallocates dev).
Indeed, I missed the free in smsusb_term_device(). Sorry about that.
Johan
