linux-kernel - Re: [PATCH v1 06/20] signal: drop else branch in do_signal

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: <20180529143039.GA1802@redhat.com>
Date:   Tue, 29 May 2018 16:30:40 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Christian Brauner <christian@...uner.io>
Cc:     linux-kernel@...r.kernel.org, ebiederm@...ssion.com,
        gregkh@...uxfoundation.org, mingo@...nel.org,
        james.morris@...rosoft.com, keescook@...omium.org,
        peterz@...radead.org, sds@...ho.nsa.gov, viro@...iv.linux.org.uk,
        akpm@...ux-foundation.org
Subject: Re: [PATCH v1 06/20] signal: drop else branch in do_signal_stop()

I am busy now, can't review, just picked a random patch from this series...

On 05/28, Christian Brauner wrote:
> do_signal_stop() already returns in the if branch so there's no need to
> keep the else branch around.

OK, but for what???

Do you think this change makes the code more readable? more clean? or what?

I do not really care but to me these "if/else" branches make this code more
symmetrical, so I don't understand the purpose.



> Signed-off-by: Christian Brauner <christian@...uner.io>
> ---
> v0->v1:
> * patch unchanged
> ---
>  kernel/signal.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index a628b56415e6..d1914439f144 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2214,14 +2214,14 @@ static bool do_signal_stop(int signr)
>  		/* Now we don't run again until woken by SIGCONT or SIGKILL */
>  		freezable_schedule();
>  		return true;
> -	} else {
> -		/*
> -		 * While ptraced, group stop is handled by STOP trap.
> -		 * Schedule it and let the caller deal with it.
> -		 */
> -		task_set_jobctl_pending(current, JOBCTL_TRAP_STOP);
> -		return false;
>  	}
> +
> +	/*
> +	 * While ptraced, group stop is handled by STOP trap.
> +	 * Schedule it and let the caller deal with it.
> +	 */
> +	task_set_jobctl_pending(current, JOBCTL_TRAP_STOP);
> +	return false;
>  }
>  
>  /**
> -- 
> 2.17.0
>