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;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Gautam Menghani <gautam@linux.ibm.com>, npiggin@gmail.com,
        christophe.leroy@csgroup.eu
Cc:     Gautam Menghani <gautam@linux.ibm.com>,
        linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code
 in XICS
In-Reply-To: <20230630055628.17790-1-gautam@linux.ibm.com>
References: <20230630055628.17790-1-gautam@linux.ibm.com>
Date:   Mon, 03 Jul 2023 16:41:41 +1000
Message-ID: <878rbxwl7e.fsf@mail.lhotse>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Gautam Menghani <gautam@linux.ibm.com> writes:
> Remove an unnecessary piece of code that does an endianness conversion but
> does not use the result. The following warning was reported by Clang's
> static analyzer:
>
> arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> 'server' is never read [deadcode.DeadStores]
>         server = be16_to_cpu(oserver);
>
> As the result of endianness conversion is never used, delete the line
> and fix the warning.
>
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  arch/powerpc/sysdev/xics/ics-opal.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> index 6cfbb4fac7fb..5fe73dabab79 100644
> --- a/arch/powerpc/sysdev/xics/ics-opal.c
> +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
>  		       __func__, d->irq, hw_irq, rc);
>  		return -1;
>  	}
> -	server = be16_to_cpu(oserver);
>  
>  	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
>  	if (wanted_server < 0) {

My first question with a patch like this is always going to be "how did
the code end up like this?"

Has the code changed and this assignment became unused? If so the commit
that did that should be identified.

If the code has always been like this that's also useful to know. Or
something else happened for it to end up this way :)

The second question will be "is there actually a bug here?". ie. should
server actually be used, and the bug is not that it's a dead assignment
but rather that server is not where it should be.

cheers