Re: [PATCH] media: usb: dvb-usb-v2: clean the freed pointer and counter

[Sean Young <sean@mess.org>]

On Sun, Aug 15, 2021 at 06:01:09PM +0800, Nil Yi wrote:
> After urb was freed, the pointer and counter need to be
> cleaned.

Ideally the commit message will say why this change needs happen, i.e.
what code path might lead to something bad happening.

I'm re-reading the code and I'm having a hard time figuring out what
that code path is, i.e. why this patch is needed.

Also "cleaned" isn't common terminology as far I know. Better to avoid
that.

Thanks

Sean

> 
> Signed-off-by: Nil Yi <teroincn@163.com>
> ---
>  drivers/media/usb/dvb-usb-v2/usb_urb.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/usb_urb.c b/drivers/media/usb/dvb-usb-v2/usb_urb.c
> index 2ad2ddeaf..08ba2d138 100644
> --- a/drivers/media/usb/dvb-usb-v2/usb_urb.c
> +++ b/drivers/media/usb/dvb-usb-v2/usb_urb.c
> @@ -143,8 +143,11 @@ static int usb_urb_alloc_bulk_urbs(struct usb_data_stream *stream)
>  		stream->urb_list[i] = usb_alloc_urb(0, GFP_ATOMIC);
>  		if (!stream->urb_list[i]) {
>  			dev_dbg(&stream->udev->dev, "%s: failed\n", __func__);
> -			for (j = 0; j < i; j++)
> +			for (j = 0; j < i; j++) {
>  				usb_free_urb(stream->urb_list[j]);
> +				stream->urb_list[j] = NULL;
> +			}
> +			stream->urbs_initialized = 0;
>  			return -ENOMEM;
>  		}
>  		usb_fill_bulk_urb(stream->urb_list[i],
> @@ -173,8 +176,11 @@ static int usb_urb_alloc_isoc_urbs(struct usb_data_stream *stream)
>  				stream->props.u.isoc.framesperurb, GFP_ATOMIC);
>  		if (!stream->urb_list[i]) {
>  			dev_dbg(&stream->udev->dev, "%s: failed\n", __func__);
> -			for (j = 0; j < i; j++)
> +			for (j = 0; j < i; j++) {
>  				usb_free_urb(stream->urb_list[j]);
> +				stream->urb_list[j] = NULL;
> +			}
> +			stream->urbs_initialized = 0;
>  			return -ENOMEM;
>  		}
>  
> -- 
> 2.17.1
>