Re: [PATCH 1/2] gnss: add USB support

From: Oliver Neukum <oneukum@suse.com>
To: Johan Hovold <johan@kernel.org>
Cc: Marc Ferland <ferlandm@amotus.ca>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] gnss: add USB support
Date: Mon, 20 Dec 2021 13:49:23 +0100	[thread overview]
Message-ID: <091d25b4-cfa5-c702-144a-8cfdb70b7f42@suse.com> (raw)
In-Reply-To: <20211220111901.23206-2-johan@kernel.org>

On 20.12.21 12:19, Johan Hovold wrote:
> +static int gnss_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	struct usb_endpoint_descriptor *in, *out;
> +	struct gnss_device *gdev;
> +	struct gnss_usb *gusb;
> +	struct urb *urb;
> +	size_t buf_len;
> +	void *buf;
> +	int ret;
> +
> +	ret = usb_find_common_endpoints(intf->cur_altsetting, &in, &out, NULL,
> +			NULL);
> +	if (ret)
> +		return ret;
> +
> +	gusb = kzalloc(sizeof(*gusb), GFP_KERNEL);
> +	if (!gusb)
> +		return -ENOMEM;
> +
> +	gdev = gnss_allocate_device(&intf->dev);
> +	if (!gdev) {
> +		ret = -ENOMEM;
> +		goto err_free_gusb;
> +	}
> +
> +	gdev->ops = &gnss_usb_gnss_ops;
> +	gdev->type = GNSS_TYPE_NMEA;
> +	gnss_set_drvdata(gdev, gusb);
> +
> +	urb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!urb)
> +			goto err_put_gdev;

The value of 'ret' here is the result of
usb_find_common_endpoints(), hence 0 due to the prior test.

> +
> +	buf_len = max(usb_endpoint_maxp(in), GNSS_USB_READ_BUF_LEN);
> +
> +	buf = kzalloc(buf_len, GFP_KERNEL);
> +	if (!buf)
> +		goto err_free_urb;
> +
> +	usb_fill_bulk_urb(urb, udev,
> +			usb_rcvbulkpipe(udev, usb_endpoint_num(in)),
> +			buf, buf_len, gnss_usb_rx_complete, gusb);
> +
> +	gusb->intf = intf;
> +	gusb->udev = udev;
> +	gusb->gdev = gdev;
> +	gusb->read_urb = urb;
> +	gusb->write_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(out));
> +
> +	ret = gnss_register_device(gdev);
> +	if (ret)
> +		goto err_free_buf;
> +
> +	usb_set_intfdata(intf, gusb);
> +
> +	return 0;
> +
> +err_free_buf:
> +	kfree(buf);
> +err_free_urb:
> +	usb_free_urb(urb);
> +err_put_gdev:
> +	gnss_put_device(gdev);
> +err_free_gusb:
> +	kfree(gusb);
> +
> +	return ret;
Yet you return it in the error case and subsequent error cases..

    HTH
        Oliver