Hi Ricardo, Thank you for the patch. On Mon, Apr 15, 2024 at 07:34:20PM +0000, Ricardo Ribalda wrote: > Avoid using the iterators after the list_for_each() constructs. > This patch should be a NOP, but makes cocci, happier: > > drivers/media/usb/uvc/uvc_ctrl.c:1861:44-50: ERROR: invalid reference to the index variable of the iterator on line 1850 > drivers/media/usb/uvc/uvc_ctrl.c:2195:17-23: ERROR: invalid reference to the index variable of the iterator on line 2179 > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index e59a463c2761..a4a987913430 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1850,16 +1850,18 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, > list_for_each_entry(entity, &chain->entities, chain) { If we really want to ensure the iterator won't be used after the loop, it could be declared in the loop statement itself, now that the kernel has switched to a newer C version. > ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback, > &err_ctrl); > - if (ret < 0) > + if (ret < 0) { > + if (ctrls) > + ctrls->error_idx = > + uvc_ctrl_find_ctrl_idx(entity, ctrls, > + err_ctrl); > goto done; > + } > } > > if (!rollback) > uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count); > done: > - if (ret < 0 && ctrls) > - ctrls->error_idx = uvc_ctrl_find_ctrl_idx(entity, ctrls, > - err_ctrl); > mutex_unlock(&chain->ctrl_mutex); > return ret; > } > @@ -2165,7 +2167,7 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, > int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > struct uvc_xu_control_query *xqry) > { > - struct uvc_entity *entity; > + struct uvc_entity *entity, *iter; > struct uvc_control *ctrl; > unsigned int i; > bool found; > @@ -2175,16 +2177,16 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > int ret; > > /* Find the extension unit. */ > - found = false; > - list_for_each_entry(entity, &chain->entities, chain) { > - if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT && > - entity->id == xqry->unit) { > - found = true; > + entity = NULL; > + list_for_each_entry(iter, &chain->entities, chain) { Same here, iter could be declared in the loop. > + if (UVC_ENTITY_TYPE(iter) == UVC_VC_EXTENSION_UNIT && > + iter->id == xqry->unit) { > + entity = iter; > break; > } > } > > - if (!found) { > + if (!entity) { > uvc_dbg(chain->dev, CONTROL, "Extension unit %u not found\n", > xqry->unit); > return -ENOENT; > -- Regards, Laurent Pinchart