Re: [RESEND PATCH v3 06/32] media: v4l: async: Clean up testing for duplicate async subdevs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On Tue, May 30, 2023 at 05:42:29AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, May 25, 2023 at 12:15:49PM +0300, Sakari Ailus wrote:
> > There's a need to verify that a single async sub-device isn't being added
> > multiple times, this would be an error. This takes place at the time of
> > adding the async sub-device to the notifier's list as well as when the
> > notifier is added to the global notifier's list.
> > 
> > Use the pointer to the sub-device for testing this instead of an index to
> > an array that is long gone. (There was an array of async sub-devices in
> > the notifier before it was converted to a linked list by commit
> > 66beb323e4a0 ("media: v4l2: async: Remove notifier subdevs array").
> 
> Unbalanced opening and closing parentheses.
> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index c5781124337af..320fe5cbaaf41 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -520,21 +520,19 @@ __v4l2_async_nf_has_async_subdev(struct v4l2_async_notifier *notifier,
> >  
> >  /*
> >   * Find out whether an async sub-device was set up already or
> > - * whether it exists in a given notifier before @this_index.
> > - * If @this_index < 0, search the notifier's entire @asd_list.
> > + * whether it exists in a given notifier.
> 
> Please document what the skip_self parameter does. The parameter name
> doesn't match the 'break' in the test below, I was expecting a
> 'continue'. If my expectation is wrong documentation should help, if
> it's correct, then you can fix the code :-)

I can add some, the argument will disappear later in the series though.

This patch mirrors the old functionality, nothing more. Changing the break
to continue is functionally equivalent here as this is the end of the list,
I can do that, too.

-- 
Regards,

Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux