linux-kernel - Re: [PATCH] ipc/sem: Three function calls less in do

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 6 Jul 2019 21:13:46 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Markus Elfring <Markus.Elfring@....de>,
        kernel-janitors@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Davidlohr Bueso <dave@...olabs.net>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Manfred Spraul <manfred@...orfullife.com>,
        Mathieu Malaterre <malat@...ian.org>
Cc:     LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ipc/sem: Three function calls less in do_semtimedop()

On 06/07/2019 13:28, Markus Elfring wrote:
> From: Markus Elfring <elfring@...rs.sourceforge.net>
> Date: Sat, 6 Jul 2019 14:16:24 +0200
> 
> Avoid three function calls by using ternary operators instead of
> conditional statements.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>
> ---
>  ipc/sem.c | 25 ++++++++-----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 7da4504bcc7c..56ea549ac270 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -2122,27 +2122,18 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
>  		int idx = array_index_nospec(sops->sem_num, sma->sem_nsems);
>  		curr = &sma->sems[idx];
> 
> -		if (alter) {
> -			if (sma->complex_count) {
> -				list_add_tail(&queue.list,
> -						&sma->pending_alter);
> -			} else {
> -
> -				list_add_tail(&queue.list,
> -						&curr->pending_alter);
> -			}
> -		} else {
> -			list_add_tail(&queue.list, &curr->pending_const);
> -		}
> +		list_add_tail(&queue.list,
> +			      alter
> +			      ? (sma->complex_count
> +				? &sma->pending_alter
> +				: &curr->pending_alter)
> +			      : &curr->pending_const);

Just no. This is making the code harder to comprehend with no advantage.
Compilers are smart, let the do the optimization work and keep code
simple for us mere mortals.

Colin

>  	} else {
>  		if (!sma->complex_count)
>  			merge_queues(sma);
> 
> -		if (alter)
> -			list_add_tail(&queue.list, &sma->pending_alter);
> -		else
> -			list_add_tail(&queue.list, &sma->pending_const);
> -
> +		list_add_tail(&queue.list,
> +			      alter ? &sma->pending_alter : &sma->pending_const);
>  		sma->complex_count++;
>  	}
> 
> --
> 2.22.0
>