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
