| Message ID | 20220718222757.8203-1-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: uvcvideo: Use indexed loops in uvc_ctrl_init_ctrl() | expand |
Hi Laurent it definitely looks nicer :) Thanks! On Tue, 19 Jul 2022 at 00:28, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > As shown by the bug introduced in commit 86f7ef773156 ("media: uvcvideo: > Add support for per-device control mapping overrides"), the loop style > used by uvc_ctrl_init_ctrl() is error-prone. Rewrite the loops to use > indices instead. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> > --- > This patch depends on https://lore.kernel.org/linux-media/20220718121219.16079-1-laurent.pinchart@ideasonboard.com > --- > drivers/media/usb/uvc/uvc_ctrl.c | 34 ++++++++++++++++++-------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 8c208db9600b..5c33b0b7ef9a 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2411,10 +2411,9 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev, > static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, > struct uvc_control *ctrl) > { > - const struct uvc_control_info *info = uvc_ctrls; > - const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls); > - const struct uvc_control_mapping *mapping; > - const struct uvc_control_mapping *mend; > + const struct uvc_control_mapping *mappings; > + unsigned int num_mappings; > + unsigned int i; > > /* > * XU controls initialization requires querying the device for control > @@ -2425,7 +2424,9 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, > if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT) > return; > > - for (; info < iend; ++info) { > + for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) { > + const struct uvc_control_info *info = &uvc_ctrls[i]; > + > if (uvc_entity_match_guid(ctrl->entity, info->entity) && > ctrl->index == info->index) { > uvc_ctrl_add_info(chain->dev, ctrl, info); > @@ -2452,9 +2453,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, > */ > if (chain->dev->info->mappings) { > bool custom = false; > - unsigned int i; > > - for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) { > + for (i = 0; chain->dev->info->mappings[i]; ++i) { > + const struct uvc_control_mapping *mapping = > + chain->dev->info->mappings[i]; > + > if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && > ctrl->info.selector == mapping->selector) { > __uvc_ctrl_add_mapping(chain, ctrl, mapping); > @@ -2467,10 +2470,9 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, > } > > /* Process common mappings next. */ > - mapping = uvc_ctrl_mappings; > - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings); > + for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) { > + const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i]; > > - for (; mapping < mend; ++mapping) { > if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && > ctrl->info.selector == mapping->selector) > __uvc_ctrl_add_mapping(chain, ctrl, mapping); > @@ -2478,14 +2480,16 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, > > /* Finally process version-specific mappings. */ > if (chain->dev->uvc_version < 0x0150) { > - mapping = uvc_ctrl_mappings_uvc11; > - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11); > + mappings = uvc_ctrl_mappings_uvc11; > + num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc11); > } else { > - mapping = uvc_ctrl_mappings_uvc15; > - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15); > + mappings = uvc_ctrl_mappings_uvc15; > + num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc15); > } > > - for (; mapping < mend; ++mapping) { > + for (i = 0; i < num_mappings; ++i) { > + const struct uvc_control_mapping *mapping = &mappings[i]; > + > if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && > ctrl->info.selector == mapping->selector) > __uvc_ctrl_add_mapping(chain, ctrl, mapping); > -- > Regards, > > Laurent Pinchart > -- Ricardo Ribalda
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 8c208db9600b..5c33b0b7ef9a 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -2411,10 +2411,9 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev, static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, struct uvc_control *ctrl) { - const struct uvc_control_info *info = uvc_ctrls; - const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls); - const struct uvc_control_mapping *mapping; - const struct uvc_control_mapping *mend; + const struct uvc_control_mapping *mappings; + unsigned int num_mappings; + unsigned int i; /* * XU controls initialization requires querying the device for control @@ -2425,7 +2424,9 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT) return; - for (; info < iend; ++info) { + for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) { + const struct uvc_control_info *info = &uvc_ctrls[i]; + if (uvc_entity_match_guid(ctrl->entity, info->entity) && ctrl->index == info->index) { uvc_ctrl_add_info(chain->dev, ctrl, info); @@ -2452,9 +2453,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, */ if (chain->dev->info->mappings) { bool custom = false; - unsigned int i; - for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) { + for (i = 0; chain->dev->info->mappings[i]; ++i) { + const struct uvc_control_mapping *mapping = + chain->dev->info->mappings[i]; + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && ctrl->info.selector == mapping->selector) { __uvc_ctrl_add_mapping(chain, ctrl, mapping); @@ -2467,10 +2470,9 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, } /* Process common mappings next. */ - mapping = uvc_ctrl_mappings; - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings); + for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) { + const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i]; - for (; mapping < mend; ++mapping) { if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && ctrl->info.selector == mapping->selector) __uvc_ctrl_add_mapping(chain, ctrl, mapping); @@ -2478,14 +2480,16 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, /* Finally process version-specific mappings. */ if (chain->dev->uvc_version < 0x0150) { - mapping = uvc_ctrl_mappings_uvc11; - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11); + mappings = uvc_ctrl_mappings_uvc11; + num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc11); } else { - mapping = uvc_ctrl_mappings_uvc15; - mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15); + mappings = uvc_ctrl_mappings_uvc15; + num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc15); } - for (; mapping < mend; ++mapping) { + for (i = 0; i < num_mappings; ++i) { + const struct uvc_control_mapping *mapping = &mappings[i]; + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && ctrl->info.selector == mapping->selector) __uvc_ctrl_add_mapping(chain, ctrl, mapping);
As shown by the bug introduced in commit 86f7ef773156 ("media: uvcvideo: Add support for per-device control mapping overrides"), the loop style used by uvc_ctrl_init_ctrl() is error-prone. Rewrite the loops to use indices instead. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- This patch depends on https://lore.kernel.org/linux-media/20220718121219.16079-1-laurent.pinchart@ideasonboard.com --- drivers/media/usb/uvc/uvc_ctrl.c | 34 ++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 15 deletions(-)