diff mbox series

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

Message ID 20230525091615.2324824-8-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Separate links and async sub-devices | expand

Commit Message

Sakari Ailus May 25, 2023, 9:15 a.m. UTC
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.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-async.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Laurent Pinchart May 30, 2023, 2:46 a.m. UTC | #1
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);
Sakari Ailus June 13, 2023, 2 p.m. UTC | #2
Hi Laurent,

On Tue, May 30, 2023 at 05:46:50AM +0300, Laurent Pinchart wrote:
> 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.

I think you're right, I'll drop this patch. This initialisation will be
removed in a later patch though, as the list will be redundant soon.
Sakari Ailus June 13, 2023, 2:08 p.m. UTC | #3
On Tue, Jun 13, 2023 at 02:00:43PM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> On Tue, May 30, 2023 at 05:46:50AM +0300, Laurent Pinchart wrote:
> > 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.
> 
> I think you're right, I'll drop this patch. This initialisation will be
> removed in a later patch though, as the list will be redundant soon.

Actually the list and the field remains, although it becomes unnecessary to
initialise it. I'll see if this patch would be meaningful later on in the
series or squashed to another patch.
diff mbox series

Patch

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);