Re: [RESEND PATCH v3 07/32] media: v4l: async: Drop unneeded list entry initialisation

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 07/32] media: v4l: async: Drop unneeded list entry initialisation
Date: Tue, 30 May 2023 05:46:50 +0300	[thread overview]
Message-ID: <20230530024650.GH21633@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230525091615.2324824-8-sakari.ailus@linux.intel.com>

Hi Sakari,

Thank you for the patch.

On Thu, May 25, 2023 at 12:15:50PM +0300, Sakari Ailus wrote:
> The list entry is initialised as a head in v4l2_async_register_subdev()
> just before being added to the list. This isn't needed, drop the
> initialisation.

Is this really unneeded ? Before the initialization and the list_add()
call there are a few code paths that can access the async_list. For
instance, the error path calls v4l2_async_cleanup(), which calls

	list_del_init(&sd->async_list);

That won't work well on an uninitialized (or zero-initialized)
list_head.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 320fe5cbaaf41..aef9a16e892ef 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -823,8 +823,6 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>  
>  	mutex_lock(&list_lock);
>  
> -	INIT_LIST_HEAD(&sd->async_list);
> -
>  	list_for_each_entry(notifier, &notifier_list, list) {
>  		struct v4l2_device *v4l2_dev =
>  			v4l2_async_nf_find_v4l2_dev(notifier);

-- 
Regards,

Laurent Pinchart