Hello,
On Wednesday, 28 February 2018 10:45:21 EET SF Markus Elfring wrote:
> >> +put_isp:
> >> + omap3isp_put(video->isp);
> >> +delete_fh:
> >> + v4l2_fh_del(&handle->vfh);
> >> + v4l2_fh_exit(&handle->vfh);
> >> + kfree(handle);
> >
> > Please prefix the error labels with error_.
>
> How often do you really need such an extra prefix?
>
> >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> >> @@ -994,10 +994,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
> >> void *fh, struct v4l2_queryctrl qc = { .id = ctrl->id };
> >>
> >> ret = uvc_query_v4l2_ctrl(chain, &qc);
> >>
> >> - if (ret < 0) {
> >> - ctrls->error_idx = i;
> >> - return ret;
> >> - }
> >> + if (ret < 0)
> >> + goto set_index;
> >>
> >> ctrl->value = qc.default_value;
> >>
> >> }
> >>
> >> @@ -1013,14 +1011,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file
> >> *file,
> >> void *fh, ret = uvc_ctrl_get(chain, ctrl);
> >>
> >> if (ret < 0) {
> >>
> >> uvc_ctrl_rollback(handle);
> >>
> >> - ctrls->error_idx = i;
> >> - return ret;
> >> + goto set_index;
> >>
> >> }
> >>
> >> }
> >>
> >> ctrls->error_idx = 0;
> >>
> >> return uvc_ctrl_rollback(handle);
> >>
> >> +
> >> +set_index:
> >> + ctrls->error_idx = i;
> >> + return ret;
> >>
> >> }
> >
> > For uvcvideo I find this to hinder readability
>
> I got an other development view.
>
> > without adding much added value.
>
> There can be a small effect for such a function implementation.
>
> > Please drop the uvcvideo change from this patch.
>
> Would it be nice if this source code adjustment could be integrated also?
Just for the record, and to avoid merging this patch by mistake,
Nacked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
at least until the requested changes are implemented.
--
Regards,
Laurent Pinchart
