1. Patchew
  2. linux
  3. View series

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

Desnes Nunes posted 1 patch 3 months, 1 week ago
There is a newer version of this series
drivers/media/usb/uvc/uvc_ctrl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Desnes Nunes 3 months, 1 week ago 
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@kernel.org
Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
Signed-off-by: Desnes Nunes <desnesn@redhat.com>
---
 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);
-- 
2.49.0
Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Hans de Goede 3 months ago 
Hi,

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@kernel.org
> Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
> Signed-off-by: Desnes Nunes <desnesn@redhat.com>

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/

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.

Regards,

Hans



> ---
>  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);
Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Laurent Pinchart 3 months ago 
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@kernel.org
> > Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
> > Signed-off-by: Desnes Nunes <desnesn@redhat.com>
> 
> 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@pendragon.ideasonboard.com), 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
Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Hans de Goede 3 months ago 
Hi,

On 7-Jul-25 14:50, Laurent Pinchart wrote:
> 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@kernel.org
>>> Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
>>> Signed-off-by: Desnes Nunes <desnesn@redhat.com>
>>
>> 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@pendragon.ideasonboard.com), I still prefer local
> loop variables. Can we use that please ?

Ok, I'll drop this from for-next.

Desnes, can you prepare a v3 moving the toplevel declation
inside the other loop please ?

Regards,

Hans




>> 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);
>>
>
Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Desnes Nunes 3 months ago 
Hello Hans,

On Mon, Jul 7, 2025 at 10:01 AM Hans de Goede <hansg@kernel.org> wrote:
...
> > As mentioned during the review (see
> > 20250701192928.GB16835@pendragon.ideasonboard.com), I still prefer local
> > loop variables. Can we use that please ?
>
> Ok, I'll drop this from for-next.
>
> Desnes, can you prepare a v3 moving the toplevel declation
> inside the other loop please ?

Sure, I will send a v3 with the requested changes today.

Best Regards,

-- 
Desnes Nunes
Re: [PATCH v2] media: uvcvideo: avoid variable shadowing in uvc_ctrl_cleanup_fh
Posted by Ricardo Ribalda 3 months, 1 week ago 
On Tue, 1 Jul 2025 at 19:27, Desnes Nunes <desnesn@redhat.com> 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@kernel.org
> Fixes: 10acb9101355 ("media: uvcvideo: Increase/decrease the PM counter per IOCTL")
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Desnes Nunes <desnesn@redhat.com>
> ---
>  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);
> --
> 2.49.0
>
>


-- 
Ricardo Ribalda

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.