Subject: Re: [PATCH 1/2] gnss: add USB support
To:     Johan Hovold <johan@kernel.org>
CC:     Marc Ferland <ferlandm@amotus.ca>, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20211220111901.23206-1-johan@kernel.org>
 <20211220111901.23206-2-johan@kernel.org>
From:   Oliver Neukum <oneukum@suse.com>
Message-ID: <091d25b4-cfa5-c702-144a-8cfdb70b7f42@suse.com>
Date:   Mon, 20 Dec 2021 13:49:23 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.12.0
In-Reply-To: <20211220111901.23206-2-johan@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?cTcqaB0i4WEBOmAWDfmGL0tqu/fONkARLehxqCgd5fPJAOZp1Cfb74np3fuS?=
 =?us-ascii?Q?CKcmtTjw9mAeJPxKoB6wP6+3+a4LpEh+YqA0c0n1qSEoqk5OImTgFWu/exQx?=
 =?us-ascii?Q?+INkaLLZhv7YwhDpZ+aq7qIfHsWvWUp1/KoY4otz8yd9LQusmiuMOpgmv4PV?=
 =?us-ascii?Q?An0P7k0QvtZsajFzsgjYu8DkdAy+NHTLrxM7X6esE2tQLL+x+TqgyvFkhg3V?=
 =?us-ascii?Q?OyqsoG6DCVowq/9PlW3Mvx/tKL5mbfIeuO2tVKuydSS59qWcXgZEJQFWjBdS?=
 =?us-ascii?Q?zICmmKoqmDV9Kfv3UPS181pq4PJ/3JP1vLESn+A2YrKavQr09uaQTKVm5kXt?=
 =?us-ascii?Q?7DOAinHRZMg0J+j2sgLy4L+LLs2voX8VVsNgO9aMB+lUGkpuGlHEHBNkBYX0?=
 =?us-ascii?Q?fL8TrlJAujFT6retQpP9Ql1Cu1yNz82rNFrh1jpYVhayAItA78hYA3yT8iS8?=
 =?us-ascii?Q?y1g0+PdciXeJYrM8xVaLgpc33BLEMla6xYjFleAWvnhD60VgYYDF2mcCcVFu?=
 =?us-ascii?Q?Eea07OCwoXJfVltiiwmCH/5x7c6vXoXvp9fbgo7lCECE7hRvMrA3bD7MRWNz?=
 =?us-ascii?Q?nlQiWNoTzuaS+882vdJojD4JM606DNFzyETCDsEkTo71UbfKTg6y2zu50E8H?=
 =?us-ascii?Q?WXmjiNaHpOaS5FMxzaPUMRChe7cm7OYcZDVFbbqVwqaFoUdXdX5rOGMZQXY7?=
 =?us-ascii?Q?6Adty/TeCIVqQybTyz+IGWDafKmuuRbeeLyZV71QvVMq/PtrwhX6jxhUs/CJ?=
 =?us-ascii?Q?bjUW/NCTIqMRR3ZRbN79zSlc9y6J8M6rn7FX0XzlI24Z1RT6iX7IPYL0bz0o?=
 =?us-ascii?Q?m47Sf3WCLG7OQalDHzawI3WqtTc55eYM9ouGKLkeIZ+Tgr6iZTqRbvHknIRe?=
 =?us-ascii?Q?lqUhO5XT5mA0ZuIr1AWvT3PbPLpX14Ji3+ok9b9b5n7F51nUMkEeKICecTja?=
 =?us-ascii?Q?WRbybQajAhqLvfKhfTR3ho/r/Xfor7PF8Qy33Vqtu1N3unRPetupu+s2hf2C?=
 =?us-ascii?Q?soWpt8u3ibDzdbRg0mp9YL0zwu9R7od96OaElPE5upPIsEA/ymNFuBFxqqKO?=
 =?us-ascii?Q?++Jl+lLO1Gerf/eIJLOCF5z+2VaR0jjsTFdTIwZ1nkFO03gOaOv+5dg11NLo?=
 =?us-ascii?Q?f+fzBd4M8aaUOtJEN16rKsI5N6chB0NDJoc4TJyyUbeC3VtjWzOcgnKgM9cH?=
 =?us-ascii?Q?PbYQd1jvYZzLUfTkwPjPr0kIh8EYM8l4gZNuGRAHf0L3tMXKym1GSLILqRy0?=
 =?us-ascii?Q?IBXbguiUmWJ/LTgaY9l3+AU39eHwQHUL3al/cfM5CrWAgaWMXjqjoZZdeA6w?=
 =?us-ascii?Q?JqnIF8fT6D21pMjzY/eoMuHRobut1T3FGDylz7PpnpP77AbfOO4whXGNkN0A?=
 =?us-ascii?Q?GCvklPSGKQ+8bcYd+xgXauZQCy3Bm1WlWuivgTPRvlu3KAq5n/1zADd+kr99?=
 =?us-ascii?Q?l499BAyyE+l8HsM7ac6JiPHAVCLaV1a9rxdYnPy+LT+mLRzc2hwkqmtxNZbY?=
 =?us-ascii?Q?1Bx2DedAKSCE0jjBhT7DxUEmcMtqhb3HfNbhu7Pepg8/DxNmhpI4K87dWf8f?=
 =?us-ascii?Q?NNjaqOTQ2+RsrySmFazVwp60HlKayqcPxxYW2xJThbZE5zDmUFZXqcrfVWVZ?=
 =?us-ascii?Q?+Q7qoV9+cH2YEM/4QirHOVPz24/ObnhiNI4kZL63ju+bZhf4PD8JkeZpJAil?=
 =?us-ascii?Q?ecUQ//3Gc8CHp6EMp1Epash6jes=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 37717fee-cfc4-48fd-67a6-08d9c3b72701
X-MS-Exchange-CrossTenant-AuthSource: DB7PR04MB5050.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2021 12:49:25.2809
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dltaDbH/H/E0z7ZYjpvI1cIhGcKdx5wk/FgmvzGZ+klMOMj5cDdh4eQtMWiIquu5+CWac7f9wpcpW+r/F5JRaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB5179
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 20.12.21 12:19, Johan Hovold wrote:
> +static int gnss_usb_probe(struct usb_interface *intf, const struct usb_d=
evice_id *id)
> +{
> +	struct usb_device *udev =3D 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 =3D usb_find_common_endpoints(intf->cur_altsetting, &in, &out, NULL=
,
> +			NULL);
> +	if (ret)
> +		return ret;
> +
> +	gusb =3D kzalloc(sizeof(*gusb), GFP_KERNEL);
> +	if (!gusb)
> +		return -ENOMEM;
> +
> +	gdev =3D gnss_allocate_device(&intf->dev);
> +	if (!gdev) {
> +		ret =3D -ENOMEM;
> +		goto err_free_gusb;
> +	}
> +
> +	gdev->ops =3D &gnss_usb_gnss_ops;
> +	gdev->type =3D GNSS_TYPE_NMEA;
> +	gnss_set_drvdata(gdev, gusb);
> +
> +	urb =3D 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 =3D max(usb_endpoint_maxp(in), GNSS_USB_READ_BUF_LEN);
> +
> +	buf =3D 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 =3D intf;
> +	gusb->udev =3D udev;
> +	gusb->gdev =3D gdev;
> +	gusb->read_urb =3D urb;
> +	gusb->write_pipe =3D usb_sndbulkpipe(udev, usb_endpoint_num(out));
> +
> +	ret =3D 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..

=C2=A0=C2=A0=C2=A0 HTH
=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 Oliver