netdev - RE: [patch net-next 0/3] net/sched: Improve getting objects by indexes

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] [day] [month] [year] [list]

Date:   Wed, 16 Aug 2017 14:28:50 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Christian König' <christian.koenig@....com>,
        "Jiri Pirko" <jiri@...nulli.us>
CC:     Chris Mi <chrism@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [patch net-next 0/3] net/sched: Improve getting objects by
 indexes

From: Christian König
> Sent: 16 August 2017 09:32
> Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
> > Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@....com wrote:
> >> Am 16.08.2017 um 04:12 schrieb Chris Mi:
...
> >>> -	ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> >>> -	if (ret < 0) {
> >>> +	ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> >>> +			GFP_KERNEL);
> >>> +	if (ret) {
> >>>    		if (ret == -ENOSPC) {
> >>>    			printk(KERN_ERR "bsg: too many bsg devices\n");
> >>>    			ret = -EINVAL;
> >> The condition "if (ret)" will now always be true after the first allocation
> >> and so we always run into the error handling after that.
> > On success, idr_alloc returns 0.
> 
> Ah, I see. You change the idr_alloc to return the resulting index as
> separate parameter.

Returning values by reference typically generates considerably worse code
that using the function return value.
It isn't just the extra parameter, it can constrain the generated code
in other ways.
That is why ERR_PTR() and friends exist.
IMHO You need a really good reason to make this change.

	David