| Message ID | 20200807083548.204360-4-dwlsalmeida@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] media: sunxi: sun8i-rotate.c: remove useless error message | expand |
Hi Daniel, Thank you for the patch. On Fri, Aug 07, 2020 at 05:35:31AM -0300, Daniel W. S. Almeida wrote: > From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> > > Fixes the following coccinelle reports: > > drivers/media/usb/uvc/uvc_v4l2.c:840:8-13: > ERROR: invalid reference to the index variable of the iterator on line 836 > > drivers/media/usb/uvc/uvc_v4l2.c:851:5-10: > ERROR: invalid reference to the index variable of the iterator on line 843 > > drivers/media/usb/uvc/uvc_v4l2.c:851:22-27: > ERROR: invalid reference to the index variable of the iterator on line 843 > > Byy introducing a temporary variable for list iteration. > > Found using - Coccinelle (http://coccinelle.lip6.fr) > > Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com> > --- > drivers/media/usb/uvc/uvc_v4l2.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 0335e69b70ab..7205ef13c2e1 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -826,6 +826,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, > struct uvc_video_chain *chain = handle->chain; > const struct uvc_entity *selector = chain->selector; > struct uvc_entity *iterm = NULL; > + struct uvc_entity *cursor = NULL; > u32 index = input->index; > int pin = 0; > > @@ -833,18 +834,22 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) { > if (index != 0) > return -EINVAL; > - list_for_each_entry(iterm, &chain->entities, chain) { > - if (UVC_ENTITY_IS_ITERM(iterm)) > + list_for_each_entry(cursor, &chain->entities, chain) { > + if (UVC_ENTITY_IS_ITERM(cursor)){ > + iterm = cursor; > break; > + } > } If no entity is found by this loop the next line will dereference a NULL pointer. There's a similar issue below. I think the issues reported by coccinelle are false positives, as every chain is guaranteed to have the proper input terminals (ITERM). This is verified when constructing the chain at probe time. > pin = iterm->id; > } else if (index < selector->bNrInPins) { > pin = selector->baSourceID[index]; > - list_for_each_entry(iterm, &chain->entities, chain) { > - if (!UVC_ENTITY_IS_ITERM(iterm)) > + list_for_each_entry(cursor, &chain->entities, chain) { > + if (!UVC_ENTITY_IS_ITERM(cursor)) > continue; > - if (iterm->id == pin) > + if (cursor->id == pin) { > + iterm = cursor; > break; > + } > } > } > > @@ -1519,4 +1524,3 @@ const struct v4l2_file_operations uvc_fops = { > .get_unmapped_area = uvc_v4l2_get_unmapped_area, > #endif > }; > -
Hi Laurent, thanks for reviewing! > If no entity is found by this loop the next line will dereference a NULL > pointer. There's a similar issue below. You mean this line, right? > pin = iterm->id; In which case, yes I missed it! > I think the issues reported by coccinelle are false positives, as every > chain is guaranteed to have the proper input terminals (ITERM). This is > verified when constructing the chain at probe time. I thought so too, but I thought it would be nice to get rid of the warning anyways. Probably other people will send patches for this same issue in the future too, i.e. until this warning is not there anymore. Do you have any suggestions or should we just leave this alone? :) Maybe: if (iterm) pin = iterm->id This line will return an error if the iterm is not found anyway: if (iterm == NULL || iterm->id != pin) return -EINVAL Let me know what you think - Daniel
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 0335e69b70ab..7205ef13c2e1 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -826,6 +826,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, struct uvc_video_chain *chain = handle->chain; const struct uvc_entity *selector = chain->selector; struct uvc_entity *iterm = NULL; + struct uvc_entity *cursor = NULL; u32 index = input->index; int pin = 0; @@ -833,18 +834,22 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh, (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) { if (index != 0) return -EINVAL; - list_for_each_entry(iterm, &chain->entities, chain) { - if (UVC_ENTITY_IS_ITERM(iterm)) + list_for_each_entry(cursor, &chain->entities, chain) { + if (UVC_ENTITY_IS_ITERM(cursor)){ + iterm = cursor; break; + } } pin = iterm->id; } else if (index < selector->bNrInPins) { pin = selector->baSourceID[index]; - list_for_each_entry(iterm, &chain->entities, chain) { - if (!UVC_ENTITY_IS_ITERM(iterm)) + list_for_each_entry(cursor, &chain->entities, chain) { + if (!UVC_ENTITY_IS_ITERM(cursor)) continue; - if (iterm->id == pin) + if (cursor->id == pin) { + iterm = cursor; break; + } } } @@ -1519,4 +1524,3 @@ const struct v4l2_file_operations uvc_fops = { .get_unmapped_area = uvc_v4l2_get_unmapped_area, #endif }; -