Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 11 Apr 2022 08:47:37 -0700
From:   Nathan Chancellor <nathan@kernel.org>
To:     Michal Orzel <michalorzel.eng@gmail.com>
Cc:     Nick Desaulniers <ndesaulniers@google.com>,
        Tom Rix <trix@redhat.com>, linux-kernel@vger.kernel.org,
        llvm@lists.linux.dev, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] ipc/sem: Remove redundant assignments
Message-ID: <YlRNmXucZwNasoFq@thelio-3990X>
References: <20220409101933.207157-1-michalorzel.eng@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220409101933.207157-1-michalorzel.eng@gmail.com>
Precedence: bulk

Hi Michal,

On Sat, Apr 09, 2022 at 12:19:31PM +0200, Michal Orzel wrote:
> Get rid of redundant assignments which end up in values not being
> read either because they are overwritten or the function ends.
> 
> Reported by clang-tidy [deadcode.DeadStores]
> 
> Signed-off-by: Michal Orzel <michalorzel.eng@gmail.com>

Thank you for the patch! This should go through Andrew Morton, now CC'd.

For the future, having some history behind the origin of these warnings
would make review a little easier. I'll comment on each below.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  ipc/sem.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 0dbdb98fdf2d..38ef91a63edd 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -766,7 +766,6 @@ static int perform_atomic_semop(struct sem_array *sma, struct sem_queue *q)
>  	for (sop = sops; sop < sops + nsops; sop++) {
>  		curr = &sma->sems[sop->sem_num];
>  		sem_op = sop->sem_op;
> -		result = curr->semval;

Caused by commit 4ce33ec2e42d ("ipc/sem: optimize
perform_atomic_semop()"). Seems like a simple copy and paste bug.

>  		if (sop->sem_flg & SEM_UNDO) {
>  			int undo = un->semadj[sop->sem_num] - sem_op;
> @@ -1430,7 +1429,6 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
>  	if (err)
>  		goto out_rcu_wakeup;
>  
> -	err = -EACCES;

I was not able to figure out this one but it does look correct, as err
is either set properly in the switch statement below or we skip over it
to the 'err = -EINVAL'.

>  	switch (cmd) {
>  	case GETALL:
>  	{
> -- 
> 2.25.1
>