Re: [RESEND PATCH v3 15/32] media: v4l: async: Clean up error handling in v4l2_async_match_notify

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org,
	Philipp Zabel <p.zabel@pengutronix.de>,
	hverkuil@xs4all.nl, Francesco Dolcini <francesco@dolcini.it>,
	aishwarya.kothari@toradex.com, Robert Foss <rfoss@kernel.org>,
	Todor Tomov <todor.too@gmail.com>,
	Hyun Kwon <hyun.kwon@xilinx.com>,
	bingbu.cao@intel.com, niklas.soderlund@ragnatech.se,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Benjamin Mugnier <benjamin.mugnier@foss.st.com>,
	Sylvain Petinot <sylvain.petinot@foss.st.com>,
	Eugen Hristev <eugen.hristev@collabora.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Maxime Ripard <mripard@kernel.org>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Fabio Estevam <festevam@gmail.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Dafna Hirschfeld <dafna@fastmail.com>,
	Hugues Fruchet <hugues.fruchet@foss.st.com>,
	Yong Deng <yong.deng@magewell.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Benoit Parrot <bparrot@ti.com>,
	Steve Longerbeam <slongerbeam@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	Marco Felsch <m.felsch@pengutronix.de>
Subject: Re: [RESEND PATCH v3 15/32] media: v4l: async: Clean up error handling in v4l2_async_match_notify
Date: Tue, 30 May 2023 08:52:24 +0300	[thread overview]
Message-ID: <20230530055224.GT21633@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230525091615.2324824-16-sakari.ailus@linux.intel.com>

Hi Sakari,

Thank you for the patch.

On Thu, May 25, 2023 at 12:15:58PM +0300, Sakari Ailus wrote:
> Add labels for error handling instead of doing it all in individual cases.
> Prepare for more functionality in this function.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index b1025dfc27a92..f51f0c37210c9 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -320,10 +320,8 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
>  		return ret;
>  
>  	ret = v4l2_async_nf_call_bound(notifier, sd, asc);
> -	if (ret < 0) {
> -		v4l2_device_unregister_subdev(sd);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto err_unregister_subdev;
>  
>  	/*
>  	 * Depending of the function of the entities involved, we may want to
> @@ -332,11 +330,8 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
>  	 * pad).
>  	 */
>  	ret = v4l2_async_create_ancillary_links(notifier, sd);
> -	if (ret) {
> -		v4l2_async_nf_call_unbind(notifier, sd, asc);
> -		v4l2_device_unregister_subdev(sd);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_call_unbind;
>  
>  	list_del(&asc->waiting_entry);
>  	sd->asd = asc;
> @@ -363,6 +358,14 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
>  	subdev_notifier->parent = notifier;
>  
>  	return v4l2_async_nf_try_all_subdevs(subdev_notifier);

Unrelated to this patch, but shoulnd't this have error handling too ?

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> +err_call_unbind:
> +	v4l2_async_nf_call_unbind(notifier, sd, asc);
> +
> +err_unregister_subdev:
> +	v4l2_device_unregister_subdev(sd);
> +
> +	return ret;
>  }
>  
>  /* Test all async sub-devices in a notifier for a match. */

-- 
Regards,

Laurent Pinchart