linux-kernel - Re: [v2] [media] Use common error handling code in 20 functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <783e7eff-2028-72be-b83c-77fc4340484e@users.sourceforge.net>
Date:   Wed, 28 Feb 2018 09:45:21 +0100
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        linux-media@...r.kernel.org
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Andi Shyti <andi.shyti@...sung.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrey Utkin <andrey_utkin@...tmail.com>,
        Arvind Yadav <arvind.yadav.cs@...il.com>,
        Bhumika Goyal <bhumirks@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Brian Johnson <brijohn@...il.com>,
        Christoph Böhmwalder <christoph@...hmwalder.at>,
        Christophe Jaillet <christophe.jaillet@...adoo.fr>,
        Colin Ian King <colin.king@...onical.com>,
        Daniele Nicolodi <daniele@...nta.net>,
        David Härdeman <david@...deman.nu>,
        Devendra Sharma <devendra.sharma9091@...il.com>,
        "Gustavo A. R. Silva" <garsilva@...eddedor.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Inki Dae <inki.dae@...sung.com>, Joe Perches <joe@...ches.com>,
        Kees Cook <keescook@...omium.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Max Kellermann <max.kellermann@...il.com>,
        Mike Isely <isely@...ox.com>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Santosh Kumar Singh <kumar.san1093@...il.com>,
        Satendra Singh Thakur <satendra.t@...sung.com>,
        Sean Young <sean@...s.org>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        Shyam Saini <mayhs11saini@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Todor Tomov <todor.tomov@...aro.org>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [v2] [media] Use common error handling code in 20 functions

>> +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?

Regards,
Markus