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

[Hans de Goede <hansg@kernel.org>]

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