Hello Markus,
On Monday, 19 February 2018 20:11:56 EET SF Markus Elfring wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Mon, 19 Feb 2018 18:50:40 +0100
>
> Adjust jump targets so that a bit of exception handling can be better
> reused at the end of these functions.
>
> This issue was partly detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>
> v2:
> Hans Verkuil insisted on patch squashing. Thus several changes
> were recombined based on source files from Linux next-20180216.
>
> The implementation of the function "tda8261_set_params" was improved
> after a notification by Christoph Böhmwalder on 2017-09-26.
>
> drivers/media/dvb-core/dmxdev.c | 16 ++++----
> drivers/media/dvb-frontends/tda1004x.c | 20 ++++++----
> drivers/media/dvb-frontends/tda8261.c | 19 ++++++----
> drivers/media/pci/bt8xx/dst.c | 19 ++++++----
> drivers/media/pci/bt8xx/dst_ca.c | 30 +++++++--------
> drivers/media/pci/cx88/cx88-input.c | 17 +++++----
> drivers/media/platform/omap3isp/ispvideo.c | 29 +++++++--------
> .../media/platform/qcom/camss-8x16/camss-csid.c | 20 +++++-----
> drivers/media/tuners/tuner-xc2028.c | 30 +++++++--------
> drivers/media/usb/cpia2/cpia2_usb.c | 13 ++++---
> drivers/media/usb/gspca/gspca.c | 17 +++++----
> drivers/media/usb/gspca/sn9c20x.c | 17 +++++----
> drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 +++--
> drivers/media/usb/tm6000/tm6000-cards.c | 7 ++--
> drivers/media/usb/tm6000/tm6000-dvb.c | 11 ++++--
> drivers/media/usb/tm6000/tm6000-video.c | 13 ++++---
> drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 +++----
> drivers/media/usb/ttusb-dec/ttusb_dec.c | 43 +++++++------------
> drivers/media/usb/uvc/uvc_v4l2.c | 13 ++++---
> 19 files changed, 180 insertions(+), 177 deletions(-)
[snip]
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index
> a751c89a3ea8..2ddcac736402 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1315,14 +1315,12 @@ static int isp_video_open(struct file *file)
> /* If this is the first user, initialise the pipeline. */
> if (omap3isp_get(video->isp) == NULL) {
> ret = -EBUSY;
> - goto done;
> + goto delete_fh;
> }
>
> ret = v4l2_pipeline_pm_use(&video->video.entity, 1);
> - if (ret < 0) {
> - omap3isp_put(video->isp);
> - goto done;
> - }
> + if (ret < 0)
> + goto put_isp;
>
> queue = &handle->queue;
> queue->type = video->type;
> @@ -1335,10 +1333,8 @@ static int isp_video_open(struct file *file)
> queue->dev = video->isp->dev;
>
> ret = vb2_queue_init(&handle->queue);
> - if (ret < 0) {
> - omap3isp_put(video->isp);
> - goto done;
> - }
> + if (ret < 0)
> + goto put_isp;
>
> memset(&handle->format, 0, sizeof(handle->format));
> handle->format.type = video->type;
> @@ -1346,14 +1342,15 @@ static int isp_video_open(struct file *file)
>
> handle->video = video;
> file->private_data = &handle->vfh;
> + goto exit;
No need for a goto here, you can just return 0.
>
> -done:
> - if (ret < 0) {
> - v4l2_fh_del(&handle->vfh);
> - v4l2_fh_exit(&handle->vfh);
> - kfree(handle);
> - }
> -
> +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_.
> +exit:
> return ret;
> }
>
Otherwise the changes to omap3isp look OK to me.
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index a13ad4e178be..f64c851adea2 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ 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 without adding much added
value. Please drop the uvcvideo change from this patch.
>
> static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
--
Regards,
Laurent Pinchart
