[PATCH] x25: remove redundant pointer dev

[PATCH] x25: remove redundant pointer dev

[Colin Ian King]

Pointer dev is being assigned a value that is never used, the assignment
and the variable are redundant and can be removed. Also replace null check
with the preferred !ptr idiom.

Cleans up clang scan warning:
net/x25/x25_proc.c:94:26: warning: Although the value stored to 'dev' is
used in the enclosing expression, the value is never actually read
from 'dev' [deadcode.DeadStores]

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
 net/x25/x25_proc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/x25/x25_proc.c b/net/x25/x25_proc.c
index 3bddcbdf2e40..91a2aade3960 100644
--- a/net/x25/x25_proc.c
+++ b/net/x25/x25_proc.c
@@ -79,7 +79,6 @@ static int x25_seq_socket_show(struct seq_file *seq, void *v)
 {
 	struct sock *s;
 	struct x25_sock *x25;
-	struct net_device *dev;
 	const char *devname;
 
 	if (v == SEQ_START_TOKEN) {
@@ -91,7 +90,7 @@ static int x25_seq_socket_show(struct seq_file *seq, void *v)
 	s = sk_entry(v);
 	x25 = x25_sk(s);
 
-	if (!x25->neighbour || (dev = x25->neighbour->dev) == NULL)
+	if (!x25->neighbour || !x25->neighbour->dev)
 		devname = "???";
 	else
 		devname = x25->neighbour->dev->name;
-- 
2.35.1

Re: [PATCH] x25: remove redundant pointer dev

[Ozgur]

On Mon, May 9, 2022 at 1:45 AM Colin Ian King <colin.i.king@gmail.com> wrote:
>
> Pointer dev is being assigned a value that is never used, the assignment
> and the variable are redundant and can be removed. Also replace null check
> with the preferred !ptr idiom.
>

Hello,

*dev pointer is device assign global linked list and shouldnt be
touched by the driver so *dev wont get any value right?
Also seems to use this while network interface is initializing because
some activation information and stats information is also kept here,
for example, open *dev will call when ifconfig is called from.

route, link, forward these inital activate and move all values with
net_device *dev?

Regards

> Cleans up clang scan warning:
> net/x25/x25_proc.c:94:26: warning: Although the value stored to 'dev' is
> used in the enclosing expression, the value is never actually read
> from 'dev' [deadcode.DeadStores]
>
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---
>  net/x25/x25_proc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/x25/x25_proc.c b/net/x25/x25_proc.c
> index 3bddcbdf2e40..91a2aade3960 100644
> --- a/net/x25/x25_proc.c
> +++ b/net/x25/x25_proc.c
> @@ -79,7 +79,6 @@ static int x25_seq_socket_show(struct seq_file *seq, void *v)
>  {
>         struct sock *s;
>         struct x25_sock *x25;
> -       struct net_device *dev;
>         const char *devname;
>
>         if (v == SEQ_START_TOKEN) {
> @@ -91,7 +90,7 @@ static int x25_seq_socket_show(struct seq_file *seq, void *v)
>         s = sk_entry(v);
>         x25 = x25_sk(s);
>
> -       if (!x25->neighbour || (dev = x25->neighbour->dev) == NULL)
> +       if (!x25->neighbour || !x25->neighbour->dev)
>                 devname = "???";
>         else
>                 devname = x25->neighbour->dev->name;
> --
> 2.35.1
>

Re: [PATCH] x25: remove redundant pointer dev

[Dan Carpenter]

On Mon, May 09, 2022 at 04:57:40AM +0400, Ozgur wrote:
> On Mon, May 9, 2022 at 1:45 AM Colin Ian King <colin.i.king@gmail.com> wrote:
> >
> > Pointer dev is being assigned a value that is never used, the assignment
> > and the variable are redundant and can be removed. Also replace null check
> > with the preferred !ptr idiom.
> >
> 
> Hello,
> 
> *dev pointer is device assign global linked list and shouldnt be
> touched by the driver so *dev wont get any value right?

Why are you talking about "*dev" instead of "dev"?

> Also seems to use this while network interface is initializing because
> some activation information and stats information is also kept here,
> for example, open *dev will call when ifconfig is called from.
> 
> route, link, forward these inital activate and move all values with
> net_device *dev?

It's not clear what you are saying...

When I review these kinds of patches I ask:
1) Does Colin's patch change run time behavior?  Obviosly not.
2) Is the current code buggy?  Sometimes when there is a static checker
   warning it indicates a typo in the code.  I do not see a bug in the
   original code before Colin's patch.
3) What was the author's original intent?  This code predates git but
   I think the "dev" was just a going to be a shorter name to type than
   "x25->neighbour->dev".

I honestly have no idea what you are saying.  At first I thought you
might be saying that this is stub code.  But that seems wrong.  Also we
do not allow stub code in the kernel.

regards,
dan carpenter

Re: [PATCH] x25: remove redundant pointer dev

[Ozgur]

On Mon, May 9, 2022 at 4:44 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Mon, May 09, 2022 at 04:57:40AM +0400, Ozgur wrote:
> > On Mon, May 9, 2022 at 1:45 AM Colin Ian King <colin.i.king@gmail.com> wrote:
> > >
> > > Pointer dev is being assigned a value that is never used, the assignment
> > > and the variable are redundant and can be removed. Also replace null check
> > > with the preferred !ptr idiom.
> > >
> >
> > Hello,
> >
> > *dev pointer is device assign global linked list and shouldnt be
> > touched by the driver so *dev wont get any value right?
>
> Why are you talking about "*dev" instead of "dev"?
>

Hi Dan,
I just realized this, i thought dev was necessary for launch functions
device and to activate.
if carries this with x25, dev is useless I understand.

> > Also seems to use this while network interface is initializing because
> > some activation information and stats information is also kept here,
> > for example, open *dev will call when ifconfig is called from.
> >
> > route, link, forward these inital activate and move all values with
> > net_device *dev?
>
> It's not clear what you are saying...
>
> When I review these kinds of patches I ask:
> 1) Does Colin's patch change run time behavior?  Obviosly not.
> 2) Is the current code buggy?  Sometimes when there is a static checker
>    warning it indicates a typo in the code.  I do not see a bug in the
>    original code before Colin's patch.
> 3) What was the author's original intent?  This code predates git but
>    I think the "dev" was just a going to be a shorter name to type than
>    "x25->neighbour->dev".

Actually i thought similarly because when this patch remove dev from i
thought would depend on
other source code whose x25 interface will be implemented are not ready.

I thought any x25 interface or x25route was using it.

> I honestly have no idea what you are saying.  At first I thought you
> might be saying that this is stub code.  But that seems wrong.  Also we
> do not allow stub code in the kernel.

thanks for enlightening,

Regards

> regards,
> dan carpenter
>


-- 
Ozgur Kara
Linux Kernel Developer

ozgurk@ieee.org

Re: [PATCH] x25: remove redundant pointer dev

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Sun,  8 May 2022 22:45:00 +0100 you wrote:
> Pointer dev is being assigned a value that is never used, the assignment
> and the variable are redundant and can be removed. Also replace null check
> with the preferred !ptr idiom.
> 
> Cleans up clang scan warning:
> net/x25/x25_proc.c:94:26: warning: Although the value stored to 'dev' is
> used in the enclosing expression, the value is never actually read
> from 'dev' [deadcode.DeadStores]
> 
> [...]

Here is the summary with links:
  - x25: remove redundant pointer dev
    https://git.kernel.org/netdev/net-next/c/ecd17a87eb78

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html