Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh

[Laurent Pinchart]

Hi Hans,

On Mon, Jul 07, 2025 at 02:43:09PM +0200, Hans de Goede wrote:
> On 1-Jul-25 19:25, Desnes Nunes wrote:
> > This avoids a variable loop shadowing occurring between the local loop
> > iterating through the uvc_entity's controls and the global one going
> > through the pending async controls of the file handle
> > 
> > Cc: stable@xxxxxxxxxx
> > Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
> > Signed-off-by: Desnes Nunes <desnesn@xxxxxxxxxx>
> 
> Thank you for the new version.
> 
> I've pushed this to uvc/for-next now:
> 
> https://gitlab.freedesktop.org/linux-media/users/uvc/-/commits/for-next/

As mentioned during the review (see
20250701192928.GB16835@xxxxxxxxxxxxxxxxxxxxxxxxxx), I still prefer local
loop variables. Can we use that please ?

> Note I had to manually apply the patch because of it conflicting with:
> 
> https://gitlab.freedesktop.org/linux-media/users/uvc/-/commit/2b1e8e9f7bb30204ddb42887c4e554c131f003c9
> 
> So if you want this backported to stable you will need to send
> the original version to stable your self.
> 
> I plan to send a pull-request with the changes from for-next this Thursday.
> 
> > ---
> >  drivers/media/usb/uvc/uvc_ctrl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 44b6513c5264..532615d8484b 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -3260,7 +3260,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
> >  void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
> >  {
> >  	struct uvc_entity *entity;
> > -	int i;
> > +	unsigned int i;
> >  
> >  	guard(mutex)(&handle->chain->ctrl_mutex);
> >  
> > @@ -3268,7 +3268,7 @@ void uvc_ctrl_cleanup_fh(struct uvc_fh *handle)
> >  		return;
> >  
> >  	list_for_each_entry(entity, &handle->chain->dev->entities, list) {
> > -		for (unsigned int i = 0; i < entity->ncontrols; ++i) {
> > +		for (i = 0; i < entity->ncontrols; ++i) {
> >  			if (entity->controls[i].handle != handle)
> >  				continue;
> >  			uvc_ctrl_set_handle(handle, &entity->controls[i], NULL);
> 

-- 
Regards,

Laurent Pinchart